Build scripts review #3021
Build scripts review #3021
Conversation
WalkthroughThis update extensively modernizes Gradle Kotlin DSL build scripts across the project. It replaces string-based and eagerly evaluated IntelliJ platform configuration with provider-based, typed, and lazily evaluated constructs. Custom sandbox normalization logic and related task configurations are removed. Dependency and plugin module declarations are consolidated within Changes
Sequence Diagram(s)sequenceDiagram
participant Gradle
participant ProjectBuildScript
participant IntelliJPlatformPlugin
Gradle->>ProjectBuildScript: Evaluate build.gradle.kts
ProjectBuildScript->>IntelliJPlatformPlugin: Call intellijPlatform.create(type, version, useInstaller) using providers
IntelliJPlatformPlugin-->>ProjectBuildScript: Configure platform, modules, plugins lazily
ProjectBuildScript->>Gradle: Register tasks using delegated properties and providers
Gradle-->>ProjectBuildScript: Tasks and dependencies resolved at execution
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
♻️ Duplicate comments (21)
mojo/frontend/build.gradle.kts (1)
29-33: Same ambiguity & missing default as aboveMirror the fix applied to
embedded/common: qualify the provider factory and makeuseInstallersafe.- type = provider { IntelliJPlatformType.IntellijIdeaCommunity }, - useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, + type = providers.provider { IntelliJPlatformType.IntellijIdeaCommunity }, + useInstaller = providers.gradleProperty("useInstaller") + .map { it.toBoolean() } + .orElse(false),plugin/plenv/build.gradle.kts (1)
22-26: Clarify provider call & safeguarduseInstallerSame concern as in other modules – qualify the provider factory and supply a default value.
- type = provider { IntelliJPlatformType.IntellijIdeaCommunity }, - useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, + type = providers.provider { IntelliJPlatformType.IntellijIdeaCommunity }, + useInstaller = providers.gradleProperty("useInstaller") + .map { it.toBoolean() } + .orElse(false),embedded/frontend/build.gradle.kts (1)
29-33: Qualify provider factory & defaultuseInstallerRepeat the fix applied in other scripts:
- type = provider { IntelliJPlatformType.IntellijIdeaCommunity }, - useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, + type = providers.provider { IntelliJPlatformType.IntellijIdeaCommunity }, + useInstaller = providers.gradleProperty("useInstaller") + .map { it.toBoolean() } + .orElse(false),plugin/perlInstall/build.gradle.kts (1)
22-26: Provider qualification & safe defaultApply the same pattern:
- type = provider { IntelliJPlatformType.IntellijIdeaCommunity }, - useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, + type = providers.provider { IntelliJPlatformType.IntellijIdeaCommunity }, + useInstaller = providers.gradleProperty("useInstaller") + .map { it.toBoolean() } + .orElse(false),tt2/common/build.gradle.kts (1)
28-32: SameuseInstallerdefault-value concern as inmason2/common
See earlier comment – the identical fix applies here.plugin/frontend/split/build.gradle.kts (1)
29-33: SameuseInstallerdefault-value concern as inmason2/common
See earlier comment – the identical fix applies here.mason/mason2/frontend/split/build.gradle.kts (1)
31-35: SameuseInstallerdefault-value concern as inmason2/common
See earlier comment – the identical fix applies here.tt2/frontend/build.gradle.kts (1)
29-33: SameuseInstallerdefault-value concern as inmason2/common
See earlier comment – the identical fix applies here.plugin/copyright/build.gradle.kts (1)
22-26: SameuseInstallerfallback is missing here as well
See the earlier comment inplugin/makeMaker/build.gradle.kts.plugin/wsl/build.gradle.kts (1)
23-26: SameuseInstallerpitfall as in other scriptsApply the guarded
.orElse(false)pattern here as well to avoid runtime failures.plugin/carton/build.gradle.kts (1)
22-26: Guard against missinguseInstallerpropertySame fix as suggested for
berrybrew: add.orElse(false)after themap { … }.plugin/cpanminus/build.gradle.kts (1)
22-26: Potential failure on absentuseInstallerGuard with
.orElse(false)as discussed above.tt2/build.gradle.kts (1)
32-36: UnprotecteduseInstallermappingAdd
.orElse(false)to avoidNoSuchElementExceptionwhen the flag is not supplied.plugin/profiler/build.gradle.kts (1)
23-26: Repeat of enum-casing anduseInstallerconcerns.The same enum-name and missing-property caveats raised for
plugin/perlbrewapply here.mason/htmlmason/build.gradle.kts (1)
33-37: Enum constant &useInstallersafeguards.Please apply the enum-name verification and
orElse(false)default here as well for consistency and robustness.mason/mason2/build.gradle.kts (1)
31-37: SameuseInstallerprovider issue as noted inplugin/ideaPlease apply the
.orElse(false)safeguard here as well.plugin/moduleBuild/build.gradle.kts (2)
22-26: Missing default foruseInstallerIdentical to
plugin/idea; add.orElse(false).
28-34:compileOnly/testCompileOnlyincorrectly scopedMove them outside the
intellijPlatformblock (see detailed diff inplugin/idea).plugin/terminal/build.gradle.kts (2)
22-26:useInstallerdefault handlingSame absence-of-default problem – add
.orElse(false).
30-36: Configuration scope forcompileOnly/testCompileOnlySame scoping issue – relocate out of
intellijPlatform.mojo/build.gradle.kts (1)
31-35:useInstallerprovider needs a fallbackApply
.orElse(false)like in earlier comment.
🧹 Nitpick comments (22)
plugin/plenv/build.gradle.kts (1)
28-35: RedundantcompileOnly/testCompileOnlyalongsidepluginModule
pluginModule(project(it))already attaches the project as a compile-time dependency; addingcompileOnlyandtestCompileOnlyduplicates the class-path entry and slightly slows dependency resolution.- compileOnly(project(it)) - testCompileOnly(project(it)) - pluginModule(project(it)) + pluginModule(project(it)) // sufficient for compile & runtimeplugin/perlInstall/build.gradle.kts (1)
28-34: Duplicate dependency scopes
pluginModulealready covers compile/runtime; the extracompileOnly&testCompileOnlyentries are superfluous.- compileOnly(project(it)) - testCompileOnly(project(it)) - pluginModule(project(it)) + pluginModule(project(it))mojo/common/build.gradle.kts (1)
28-32: Guard against missinguseInstallerReplicate the safe-default pattern to keep behaviour uniform across modules.
- useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, + useInstaller = providers.gradleProperty("useInstaller") + .map(String::toBoolean) + .orElse(false),plugin/common/build.gradle.kts (1)
22-26: Defensive default foruseInstallerFor consistency with the other build scripts, supply a fallback when the property is not provided.
- useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, + useInstaller = providers.gradleProperty("useInstaller") + .map(String::toBoolean) + .orElse(false),plugin/makeMaker/build.gradle.kts (1)
28-34: Reduce duplication when wiring project modulesEach iteration repeats
compileOnly,testCompileOnly, andpluginModulefor the same target.
A tiny helper keeps the block concise and harder to forget to update:fun DependencyHandlerScope.intellijPlugin(projectPath: String) = project(projectPath).also { compileOnly(it); testCompileOnly(it); pluginModule(it) } listOf(":plugin.core").forEach(::intellijPlugin)plugin/copyright/build.gradle.kts (1)
30-36: Consider extracting a tiny helper for repetitive dependency wiringThe triple-call (
compileOnly,testCompileOnly,pluginModule) appears in several scripts; a shared extension would keep the build logic DRY.plugin/docker/build.gradle.kts (1)
31-37: Repeated dependency wiring – extract helperSame suggestion as in previous files to avoid three calls per project.
plugin/intelliLang/build.gradle.kts (1)
30-36: Extract helper to remove duplicate dependency declarationsThe triple call (
compileOnly,testCompileOnly,pluginModule) repeats across build scripts; consider a small extension as previously suggested.plugin/testFixtures/build.gradle.kts (1)
45-48: Provide a safe default foruseInstallerIf the
useInstallerproperty is not supplied on the command line,providers.gradleProperty("useInstaller")yields aProviderwith no value, and the build will fail at realisation time becausecreate()receives a missing Boolean.
Adding a fallback avoids that fragility.- useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, + useInstaller = providers + .gradleProperty("useInstaller") + .map(String::toBoolean) + .orElse(false),mason/htmlmason/frontend/split/build.gradle.kts (1)
31-35: AddorElse(false)to prevent missing-property crashesSame reasoning as in plugin/testFixtures: if
-PuseInstalleris omitted, the provider is empty and the build breaks. Defaulting tofalsekeeps the script robust.- useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, + useInstaller = providers + .gradleProperty("useInstaller") + .map(String::toBoolean) + .orElse(false),mason/framework/build.gradle.kts (1)
29-33: Gracefully handle an absentuseInstallerflagFor consistency with other modules—and to avoid a late-evaluation failure—supply a default value when the project property is missing.
- useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, + useInstaller = providers + .gradleProperty("useInstaller") + .map(String::toBoolean) + .orElse(false),mojo/frontend/split/build.gradle.kts (1)
31-35: DefaultuseInstallertofalseto keep the build resilientMirrors the recommendation for the other modules.
- useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, + useInstaller = providers + .gradleProperty("useInstaller") + .map(String::toBoolean) + .orElse(false),plugin/berrybrew/build.gradle.kts (1)
28-34: RedundantcompileOnly/testCompileOnlyalongsidepluginModule
pluginModule(project(it))already puts the module on the compile & test class-paths.
Keeping the extracompileOnly/testCompileOnlylines adds no value and clutters the graph.- compileOnly(project(it)) - testCompileOnly(project(it)) - pluginModule(project(it)) + pluginModule(project(it)) // sufficientplugin/wsl/build.gradle.kts (1)
30-36: Duplicate dependency declarations
compileOnly+testCompileOnlyare unnecessary whenpluginModuleis declared.
Consider trimming to keep the build faster & cleaner.plugin/carton/build.gradle.kts (1)
28-34: Remove superfluouscompileOnly/testCompileOnlylines
pluginModulealready covers them.plugin/cpanminus/build.gradle.kts (1)
28-34: Streamline dependency blockDrop the extra
compileOnly/testCompileOnlylines;pluginModulesuffices.tt2/build.gradle.kts (1)
41-48: Consider eliminating compile-time overlap
pluginModule(project(it))makes the packaged module available during compilation; any separatecompileOnly/testCompileOnlyfor the same projects (see lines 20-26) become redundant and can be removed to shorten classpaths.plugin/cpan/build.gradle.kts (1)
23-26: SameuseInstallerdefaulting issue.All scripts should share a small util that returns
providers.gradleProperty("useInstaller").map(String::toBoolean).orElse(false)to avoid copy-pasted logic and latent NPEs.mason/htmlmason/build.gradle.kts (1)
44-52: Consider compressing the packaging list for readability.Minor readability nit: the four
pluginModuleentries can be expressed as a single list variable declared at the top of the block; this keeps the block dense but comprehensible.build.gradle.kts (3)
228-230:jacocoRootReport– expensiveexecutionDataaggregation runs on every buildWalking the
coveragedirectory every time adds noticeable IO overhead in CI.
Cache the file list in aProvideror restrict it toonlyIf { withCoverage.get() }to save time when coverage is disabled.
120-121: Minor: avoidJvmTarget.fromTarget(it)allocation
italready contains the target string ("17"etc.). You can assign the string directly:jvmTarget = properties("javaTargetVersion")
fromTargetcreates an extra enum which is not necessary for compiler options.
278-279: Empty task registration – consider specifying a type for clarity
val generateLexers by registeringregisters a plainDefaultTask.
Giving it an explicit type (or at least a descriptive group / description) will help consumers and IDE navigation.val generateLexers by registering { group = "code generation" description = "Aggregates lexer generation tasks from sub-projects." }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (54)
build.gradle.kts(10 hunks)buildSrc/build.gradle.kts(2 hunks)embedded/build.gradle.kts(3 hunks)embedded/common/build.gradle.kts(2 hunks)embedded/core/build.gradle.kts(3 hunks)embedded/frontend/build.gradle.kts(3 hunks)embedded/frontend/split/build.gradle.kts(3 hunks)mason/framework/build.gradle.kts(3 hunks)mason/htmlmason/build.gradle.kts(3 hunks)mason/htmlmason/common/build.gradle.kts(2 hunks)mason/htmlmason/core/build.gradle.kts(3 hunks)mason/htmlmason/frontend/build.gradle.kts(3 hunks)mason/htmlmason/frontend/split/build.gradle.kts(3 hunks)mason/mason2/build.gradle.kts(3 hunks)mason/mason2/common/build.gradle.kts(2 hunks)mason/mason2/core/build.gradle.kts(3 hunks)mason/mason2/frontend/build.gradle.kts(3 hunks)mason/mason2/frontend/split/build.gradle.kts(3 hunks)mojo/build.gradle.kts(3 hunks)mojo/common/build.gradle.kts(2 hunks)mojo/core/build.gradle.kts(3 hunks)mojo/frontend/build.gradle.kts(3 hunks)mojo/frontend/split/build.gradle.kts(3 hunks)plugin/asdf/build.gradle.kts(2 hunks)plugin/berrybrew/build.gradle.kts(2 hunks)plugin/build.gradle.kts(2 hunks)plugin/carton/build.gradle.kts(2 hunks)plugin/common/build.gradle.kts(2 hunks)plugin/copyright/build.gradle.kts(2 hunks)plugin/core/build.gradle.kts(3 hunks)plugin/coverage/build.gradle.kts(2 hunks)plugin/cpan/build.gradle.kts(2 hunks)plugin/cpanminus/build.gradle.kts(2 hunks)plugin/debugger/build.gradle.kts(2 hunks)plugin/docker/build.gradle.kts(2 hunks)plugin/frontend/build.gradle.kts(2 hunks)plugin/frontend/split/build.gradle.kts(3 hunks)plugin/idea/build.gradle.kts(2 hunks)plugin/intelliLang/build.gradle.kts(2 hunks)plugin/makeMaker/build.gradle.kts(2 hunks)plugin/moduleBuild/build.gradle.kts(2 hunks)plugin/perlInstall/build.gradle.kts(2 hunks)plugin/perlbrew/build.gradle.kts(2 hunks)plugin/plenv/build.gradle.kts(2 hunks)plugin/profiler/build.gradle.kts(2 hunks)plugin/terminal/build.gradle.kts(2 hunks)plugin/testFixtures/build.gradle.kts(2 hunks)plugin/wsl/build.gradle.kts(2 hunks)settings.gradle.kts(1 hunks)tt2/build.gradle.kts(3 hunks)tt2/common/build.gradle.kts(2 hunks)tt2/core/build.gradle.kts(3 hunks)tt2/frontend/build.gradle.kts(3 hunks)tt2/frontend/split/build.gradle.kts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build (macos-latest)
- GitHub Check: Build (windows-latest)
- GitHub Check: Build (ubuntu-latest)
🔇 Additional comments (15)
buildSrc/build.gradle.kts (1)
2-2: Comment-only change looks goodCapitalizing “Gradle” improves consistency and professionalism.
No functional impact, nothing else to address.settings.gradle.kts (1)
2-2: Copyright-year bump acknowledgedPure comment update; no build-logic impact.
LGTM.embedded/common/build.gradle.kts (1)
28-32: ```shell
#!/bin/bash
set -eu
echo "Listing build.gradle.kts files:"
fd build.gradle.kts -t f || true
echo "Showing contents around the create call in embedded/common/build.gradle.kts:"
grep -R -n "create(" -n embedded/common/build.gradle.kts | sed -e 's/:/ -> /g' | head -n 10
echo
sed -n '1,200p' embedded/common/build.gradle.kts</details> <details> <summary>mason/mason2/frontend/build.gradle.kts (1)</summary> `29-33`: **Add default for absent `useInstaller` property** Same issue as in `plugin/asdf`: absence of the Gradle property causes provider resolution failure. Apply the `.orElse(false)` fallback. ```diff - useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, + useInstaller = providers.gradleProperty("useInstaller") + .map(String::toBoolean) + .orElse(false),embedded/frontend/split/build.gradle.kts (1)
31-35: Provide fallback foruseInstallerpropertyPrevent build failures when the flag is omitted.
- useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, + useInstaller = providers.gradleProperty("useInstaller") + .map(String::toBoolean) + .orElse(false),plugin/docker/build.gradle.kts (1)
28-30: Verify thatremoteRunPluginGradle property is always present
bundledPlugin(providers.gradleProperty("remoteRunPlugin"))will fail if the property is undefined.
Either give itorNull/orElse, or document that it’s mandatory.plugin/perlbrew/build.gradle.kts (2)
28-34: Duplicate delivery of the same artifacts.
pluginModule(project(it))already adds the module to the IDE sandbox; keepingcompileOnly/testCompileOnlyis fine, but verify that these modules are not also added throughruntimeOnlyelsewhere to prevent classpath duplication and warning spam at build time.
23-24: Check enum constant casing against the plugin version.
IntelliJPlatformType.IntellijIdeaCommunitylooks plausible, but the official enum constants have changed casing a few times (IDEA_COMMUNITY,IntellijIdeaCommunity, etc.). A mismatch will fail during the script’s class-loading phase, long before tasks run.If you are on the latest
org.jetbrains.intellij.platform.gradleplugin, please grep the classpath to confirm the exact constant:#!/usr/bin/env bash # Locate the enum constants shipped with the applied Gradle plugin fd IntelliJPlatformType ~/.gradle/caches | head -n 10 | xargs grep -n "enum class IntelliJPlatformType" -A3plugin/profiler/build.gradle.kts (1)
28-28: Confirm bundled module id.
"intellij.profiler.common"is correct for 2023.3+, but older IDEs expose it as"intellij.profiler". If you support multiple IDE baselines, guard this with version-based selection to avoid “cannot find bundled plugin” errors.plugin/coverage/build.gradle.kts (1)
30-36: Classpath duplication warning.Same note as for
perlbrew: verify that addingpluginModule(project(it))pluscompileOnly/testCompileOnlydoes not create duplicated jars in the sandbox. Gradle will warn but still package duplicates, which bloats artifact size.mojo/build.gradle.kts (1)
40-48: Packaging list looks goodUsing
pluginModulefor the Mojo sub-modules is the right choice and matches the new DSL.mason/mason2/core/build.gradle.kts (1)
55-59: Missing default fortemplating_lexer_skeletonpropertyIf the Gradle property is not provided, the
mapwill yield an unresolved provider and the
lexer task will fail when realised. Add a sensible default or useorElse.- skeleton = providers.gradleProperty("templating_lexer_skeleton").map { rootProject.file(it) } + skeleton = providers + .gradleProperty("templating_lexer_skeleton") + .map(rootProject::file) + .orElse(rootProject.file("gradle/lexer/templating.skeleton"))plugin/core/build.gradle.kts (1)
52-53: Possible typo –bundledModulevsbundledModulesAll other build scripts in this PR use the plural
bundledModules(...).
Unless you have an extension that adds a singular form, Gradle will fail with “unresolved reference” here.- bundledModule("intellij.spellchecker") + bundledModules("intellij.spellchecker")Please double-check the API or align with the plural form used elsewhere.
plugin/build.gradle.kts (1)
31-48: Provider passed where a plain list is expected – verifyplugins(pluginList)&bundledPlugins(bundledPluginList)
plugins(...)andbundledPlugins(...)from the IntelliJ-Platform Gradle plugin historically acceptvararg StringorIterable<String>, notProvider<List<String>>.
If no matching overload exists the build will fail with a type-mismatch.Consider:
- plugins(pluginList) + plugins(pluginList.get())and
- bundledPlugins(bundledPluginList) + bundledPlugins(bundledPluginList.get())or, if the new API supports
Provider, ignore this note.build.gradle.kts (1)
60-62: Check that the declared JGit snapshot actually exists
org.eclipse.jgit:org.eclipse.jgit:7.3.0.202506031305-rlooks like a future timestamp. Make sure it is published to Maven Central or your build will break for anyone without the artifact in a custom repository.
| val platformVersionProvider: Provider<String> by rootProject.extra | ||
| create("IC", platformVersionProvider.get(), useInstaller = providers.gradleProperty("useInstaller").get().toBoolean()) | ||
|
|
||
| create( | ||
| type = provider { IntelliJPlatformType.IntellijIdeaCommunity }, | ||
| version = platformVersionProvider, | ||
| useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Provide a safe default for useInstaller to prevent build failures
providers.gradleProperty("useInstaller") returns a provider that is absent when the property is not supplied.
Attempting to access it without a fallback (map { it.toBoolean() }) will throw at resolution time.
- useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() },
+ useInstaller = providers.gradleProperty("useInstaller")
+ .map(String::toBoolean) // transform only if present
+ .orElse(false), // safe defaultThis mirrors the common JetBrains-plugin samples and keeps CI green even when the flag is omitted.
🤖 Prompt for AI Agents
In mason/htmlmason/common/build.gradle.kts around lines 26 to 33, the
useInstaller property is accessed without a safe default, causing build failures
if the property is not supplied. Fix this by providing a fallback value using
orElse("false") before mapping to boolean, ensuring the provider always has a
value and preventing resolution-time exceptions.
| intellijPlatform { | ||
| val platformVersionProvider: Provider<String> by rootProject.extra | ||
| create("IC", platformVersionProvider.get(), useInstaller = providers.gradleProperty("useInstaller").get().toBoolean()) | ||
|
|
||
| create( | ||
| type = provider { IntelliJPlatformType.IntellijIdeaCommunity }, | ||
| version = platformVersionProvider, | ||
| useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add orElse(false) for resilience
Same concern as in mason/htmlmason/common: without a default, missing -PuseInstaller causes an exception during task graph resolution.
Recommend the identical tweak:
- useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() },
+ useInstaller = providers.gradleProperty("useInstaller")
+ .map(String::toBoolean)
+ .orElse(false),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| intellijPlatform { | |
| val platformVersionProvider: Provider<String> by rootProject.extra | |
| create("IC", platformVersionProvider.get(), useInstaller = providers.gradleProperty("useInstaller").get().toBoolean()) | |
| create( | |
| type = provider { IntelliJPlatformType.IntellijIdeaCommunity }, | |
| version = platformVersionProvider, | |
| useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, | |
| ) | |
| intellijPlatform { | |
| val platformVersionProvider: Provider<String> by rootProject.extra | |
| create( | |
| type = provider { IntelliJPlatformType.IntellijIdeaCommunity }, | |
| version = platformVersionProvider, | |
| useInstaller = providers.gradleProperty("useInstaller") | |
| .map(String::toBoolean) | |
| .orElse(false), | |
| ) |
🤖 Prompt for AI Agents
In mason/htmlmason/frontend/build.gradle.kts around lines 26 to 33, the
useInstaller property is accessed via providers.gradleProperty("useInstaller")
without a default fallback, which can cause an exception if the property is
missing. Fix this by appending orElse("false") to provide a default value, then
convert it to boolean. This ensures resilience by preventing exceptions during
task graph resolution when the property is not set.
| val platformVersionProvider: Provider<String> by rootProject.extra | ||
| create("IC", platformVersionProvider.get(), useInstaller = providers.gradleProperty("useInstaller").get().toBoolean()) | ||
|
|
||
| create( | ||
| type = provider { IntelliJPlatformType.IntellijIdeaCommunity }, | ||
| version = platformVersionProvider, | ||
| useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden useInstaller handling
Apply the same safe-default pattern here to avoid fragile builds:
- useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() },
+ useInstaller = providers.gradleProperty("useInstaller")
+ .map(String::toBoolean)
+ .orElse(false),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val platformVersionProvider: Provider<String> by rootProject.extra | |
| create("IC", platformVersionProvider.get(), useInstaller = providers.gradleProperty("useInstaller").get().toBoolean()) | |
| create( | |
| type = provider { IntelliJPlatformType.IntellijIdeaCommunity }, | |
| version = platformVersionProvider, | |
| useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, | |
| ) | |
| } | |
| val platformVersionProvider: Provider<String> by rootProject.extra | |
| create( | |
| type = provider { IntelliJPlatformType.IntellijIdeaCommunity }, | |
| version = platformVersionProvider, | |
| useInstaller = providers.gradleProperty("useInstaller") | |
| .map(String::toBoolean) | |
| .orElse(false), | |
| ) | |
| } |
🤖 Prompt for AI Agents
In plugin/frontend/build.gradle.kts around lines 26 to 33, the useInstaller
property is directly converted to boolean without a safe default, which can
cause fragile builds if the property is missing or malformed. Modify the
useInstaller assignment to provide a safe default value (e.g., false) when the
Gradle property "useInstaller" is not set or empty by using a fallback or
default method on the provider before mapping to boolean.
| useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() }, | ||
| ) |
There was a problem hiding this comment.
Missing fallback when useInstaller property is absent
providers.gradleProperty("useInstaller") fails the build if the property is not supplied on the command line or via gradle.properties.
Add a defensive default to avoid unexpected resolution errors in CI / local runs.
- useInstaller = providers.gradleProperty("useInstaller").map { it.toBoolean() },
+ useInstaller = providers.gradleProperty("useInstaller")
+ .map(String::toBoolean)
+ .orElse(false),🤖 Prompt for AI Agents
In plugin/asdf/build.gradle.kts around lines 25 to 26, the useInstaller property
is set directly from providers.gradleProperty("useInstaller") which causes build
failures if the property is missing. To fix this, add a fallback default value
(e.g., false) when the property is absent by using orElse("false") before
mapping to boolean. This ensures the build does not fail due to missing property
in CI or local runs.
| listOf( | ||
| ":plugin.core", | ||
| ":plugin.perlInstall", | ||
| ).forEach { | ||
| compileOnly(project(it)) | ||
| testCompileOnly(project(it)) | ||
| pluginModule(project(it)) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
compileOnly / testCompileOnly inside intellijPlatform block is misleading
compileOnly and testCompileOnly belong to the regular Gradle dependency scopes, not to the intellijPlatform { … } DSL.
Keeping them here works only by coincidence (method-resolution from the outer dependencies scope) and couples unrelated concerns. Move them out for clarity:
- listOf(
- ":plugin.core",
- ":plugin.perlInstall",
- ).forEach {
- compileOnly(project(it))
- testCompileOnly(project(it))
- pluginModule(project(it))
- }
+ }
+}
+
+listOf(
+ ":plugin.core",
+ ":plugin.perlInstall",
+).forEach {
+ dependencies {
+ compileOnly(project(it))
+ testCompileOnly(project(it))
+ }
+ intellijPlatform {
+ pluginModule(project(it))
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In plugin/asdf/build.gradle.kts around lines 28 to 35, the compileOnly and
testCompileOnly calls are placed inside the intellijPlatform block, which is
misleading because these are standard Gradle dependency scopes and not part of
the intellijPlatform DSL. To fix this, move the compileOnly and testCompileOnly
calls outside the intellijPlatform block into the regular dependencies block to
separate concerns and improve clarity.
| rootProject.tasks.named { it == "generateLexers" }.configureEach { | ||
| dependsOn(generateHTMLMasonLexer) | ||
| } |
There was a problem hiding this comment.
Same tasks.named predicate issue as above
-rootProject.tasks.named { it == "generateLexers" }.configureEach {
- dependsOn(generateHTMLMasonLexer)
-}
+rootProject.tasks.named("generateLexers") {
+ dependsOn(generateHTMLMasonLexer)
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| rootProject.tasks.named { it == "generateLexers" }.configureEach { | |
| dependsOn(generateHTMLMasonLexer) | |
| } | |
| rootProject.tasks.named("generateLexers") { | |
| dependsOn(generateHTMLMasonLexer) | |
| } |
🤖 Prompt for AI Agents
In mason/htmlmason/core/build.gradle.kts around lines 61 to 63, the use of
tasks.named with a predicate is incorrect. Replace the predicate form with a
direct task name string argument in tasks.named, like
tasks.named("generateLexers"), to properly reference the task. Then configure
the dependency inside the configureEach block as intended.
| rootProject.tasks.named { it == "generateLexers" }.configureEach { | ||
| dependsOn(generateTT2Lexer) | ||
| } |
There was a problem hiding this comment.
Fix invalid named call to wire lexer generation task
-rootProject.tasks.named { it == "generateLexers" }.configureEach {
- dependsOn(generateTT2Lexer)
-}
+rootProject.tasks.named("generateLexers") {
+ dependsOn(generateTT2Lexer)
+}🤖 Prompt for AI Agents
In tt2/core/build.gradle.kts around lines 72 to 74, the use of the named call is
incorrect because it uses a predicate instead of the task name string. Replace
the named call argument with the exact task name string "generateLexers" to
correctly reference the task. This will properly configure the dependency on
generateTT2Lexer.
| rootProject.tasks.named { it == "generateLexers" }.configureEach { | ||
| dependsOn(generateMojoliciousLexer) | ||
| } |
There was a problem hiding this comment.
Correct named usage for Mojolicious lexer dependency
-rootProject.tasks.named { it == "generateLexers" }.configureEach {
- dependsOn(generateMojoliciousLexer)
-}
+rootProject.tasks.named("generateLexers") {
+ dependsOn(generateMojoliciousLexer)
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| rootProject.tasks.named { it == "generateLexers" }.configureEach { | |
| dependsOn(generateMojoliciousLexer) | |
| } | |
| rootProject.tasks.named("generateLexers") { | |
| dependsOn(generateMojoliciousLexer) | |
| } |
🤖 Prompt for AI Agents
In mojo/core/build.gradle.kts around lines 59 to 61, the usage of the named
function is incorrect for specifying the "generateLexers" task. Replace the
lambda with a direct string argument "generateLexers" in the named function call
to correctly reference the task by name. This will properly configure the
dependency on generateMojoliciousLexer.
| rootProject.tasks.named { it == "generateLexers" }.configureEach { | ||
| dependsOn( | ||
| generatePerlLexer, | ||
| generatePodLexer, | ||
| ) | ||
| } |
There was a problem hiding this comment.
tasks.named { … } is an invalid overload – this will not compile
TaskContainer.named() expects the task name (String / Name) or (name, type), not a lambda.
The current call tries to pass a Boolean and will explode at configuration time.
- rootProject.tasks.named { it == "generateLexers" }.configureEach {
+ rootProject.tasks.matching { it.name == "generateLexers" }.configureEach {
dependsOn(
generatePerlLexer,
generatePodLexer,
)
}Alternatively, if the task is guaranteed to exist:
rootProject.tasks.named("generateLexers") {
dependsOn(generatePerlLexer, generatePodLexer)
}🤖 Prompt for AI Agents
In plugin/core/build.gradle.kts around lines 82 to 87, the use of tasks.named
with a lambda predicate is incorrect and causes a compilation error. Replace the
call to tasks.named { it == "generateLexers" } with
tasks.named("generateLexers") to directly reference the task by name. Then
configure the dependsOn relationship inside the lambda passed to
tasks.named("generateLexers").
| rootProject.tasks.named { it == "generateLexers" }.configureEach { | ||
| dependsOn(generateEmbeddedPerlLexer) | ||
| } |
There was a problem hiding this comment.
Same tasks.named { … } misuse here
Replace the boolean predicate with either tasks.matching { it.name == "generateLexers" } or tasks.named("generateLexers") as shown in the plugin/core fix.
🤖 Prompt for AI Agents
In embedded/core/build.gradle.kts around lines 60 to 62, the code incorrectly
uses tasks.named with a boolean predicate, which is not supported. Replace
tasks.named { it == "generateLexers" } with tasks.named("generateLexers") to
correctly reference the task by name, then call configureEach on it to add the
dependency on generateEmbeddedPerlLexer.
Summary by CodeRabbit