Dev/253 #3096
Dev/253 #3096
Conversation
WalkthroughSwitches many Gradle builds from Community → Ultimate, migrates IntelliLang bundling from plugin to module, replaces a Java AdditionalLibraryRootsProvider with Kotlin, updates a few frontend method signatures to accept an optional FileType, and normalizes test distribution dependency listings to bare identifiers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Gradle as Gradle build script
participant IntelliJDSL as intellijPlatform DSL
participant Distribution as ProductDistribution
Note over Gradle,IntelliJDSL: Old flow (plugin bundling)
Gradle->>IntelliJDSL: bundledPlugins(intelliLangPlugin)
IntelliJDSL->>Distribution: include plugin artifact
Note over Gradle,IntelliJDSL #DDEEFF: New flow (module bundling)
Gradle->>IntelliJDSL: bundledModules(intelliLangModule, intelliLangBackendModule)
IntelliJDSL->>Distribution: include platform modules
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Rationale: many repetitive, low-complexity changes (platform enum swaps) combined with heterogeneous edits (Gradle DSL changes affecting bundling, Java→Kotlin provider migration, API signature changes, and multiple test fixture updates) requiring cross-file validation. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (3)
🔇 Additional comments (2)
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: 3
🧹 Nitpick comments (6)
plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java (1)
2-2: Copyright year regressedThe header went from 2025 to 2024. Update to include the current year.
- * Copyright 2015-2024 Alexandr Evstigneev + * Copyright 2015-2025 Alexandr Evstigneevplugin/common/src/main/java/com/intellij/openapi/projectRoots/impl/PerlModuleExtension.java (1)
32-32: Remove now‑unused import
BuildableRootsChangeRescanningInfois unused after the change.Apply this diff:
-import com.intellij.util.indexing.BuildableRootsChangeRescanningInfo;plugin/frontend/split/src/main/java/com/jetbrains/rdclient/actions/base/PerlBackendDelegatingActionCustomization.kt (1)
27-30: Unused parameter: fileTypeThe new parameter isn’t used; add a suppression to avoid warnings. Optionally, short‑circuit by fileType if that becomes meaningful later.
Apply this diff:
-class PerlBackendDelegatingActionCustomization : BackendDelegatingActionCustomization() { - override fun isAvailable(dataContext: DataContext, fileType: FileType?): Boolean = isAvailable(dataContext) +class PerlBackendDelegatingActionCustomization : BackendDelegatingActionCustomization() { + @Suppress("UNUSED_PARAMETER") + override fun isAvailable(dataContext: DataContext, fileType: FileType?): Boolean = isAvailable(dataContext)plugin/frontend/split/src/main/java/com/jetbrains/rdclient/editorActions/cwm/PerlFrontendEditorActionHandlerStrategyCustomizer.kt (1)
31-37: Unused parameter: fileTypeSignature updated but parameter isn’t used. Add a suppression to keep the file clean until/if it’s incorporated into the logic.
Apply this diff:
override fun getCustomStrategy( actionId: String, editor: Editor, caret: Caret?, - dataContext: DataContext, - fileType: FileType? + dataContext: DataContext, + @Suppress("UNUSED_PARAMETER") fileType: FileType? ): ActionCallStrategyKind? =plugin/build.gradle.kts (1)
30-32: Good: module-based bundling added; consider centralizing the property.The call site is fine. To avoid typos across projects, fetch the intelliLangModule once in rootProject.extra and reuse here.
- bundledModules( - providers.gradleProperty("intelliLangModule").get(), - ) + val intelliLangModule: Provider<String> by rootProject.extra + bundledModules(intelliLangModule.get())build.gradle.kts (1)
412-413: API usage check: pass Iterable vs vararg explicitly.bundledModules(bundledModuleList) relies on an overload accepting Iterable. If only vararg(String) exists in your plugin version, use the spread operator.
- bundledModules(bundledModuleList) + bundledModules(*bundledModuleList.toTypedArray())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
build.gradle.kts(2 hunks)gradle.properties(1 hunks)plugin/build.gradle.kts(1 hunks)plugin/common/src/main/java/com/intellij/openapi/projectRoots/impl/PerlModuleExtension.java(1 hunks)plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java(2 hunks)plugin/frontend/split/src/main/java/com/jetbrains/rdclient/actions/base/PerlBackendDelegatingActionCustomization.kt(1 hunks)plugin/frontend/split/src/main/java/com/jetbrains/rdclient/editorActions/cwm/PerlFrontendEditorActionHandlerStrategyCustomizer.kt(2 hunks)plugin/intelliLang/build.gradle.kts(1 hunks)plugin/intelliLang/src/main/resources/perl5.plugin.intelliLang.main.xml(1 hunks)plugin/testFixtures/build.gradle.kts(1 hunks)
⏰ 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). (3)
- GitHub Check: Build (ubuntu-latest)
- GitHub Check: Build (macos-latest)
- GitHub Check: Build (windows-latest)
🔇 Additional comments (6)
gradle.properties (3)
30-30: PsiViewer version likely mismatched to 253 — property is referencedgradle.properties:30 defines psiViewerVersion=252.23892.248 and build.gradle.kts:373 uses it to add PsiViewer to pluginList; bump to a 253‑compatible PsiViewer build or remove the pluginList entry if unused.
20-27: ```shell
#!/bin/bash
set -euo pipefailecho "=== search: literal occurrences of org.jetbrains.intellij ==="
rg -nF --hidden --no-ignore-vcs 'org.jetbrains.intellij' -S -C3 || trueecho
echo "=== search: plugin id(...) double-quoted ==="
rg -n --hidden --no-ignore-vcs 'id("org.jetbrains.intellij")' -S -C3 || trueecho
echo "=== search: plugin id(...) single-quoted ==="
rg -n --hidden --no-ignore-vcs "id('org.jetbrains.intellij')" -S -C3 || trueecho
echo "=== search: 'plugins {' blocks (context) ==="
rg -n --hidden --no-ignore-vcs 'plugins\s*{' -S -C3 || trueecho
echo "=== search: intellij / intellijPlatform occurrences ==="
rg -n --hidden --no-ignore-vcs '\bintellij\b|\bintellijPlatform\b' -S -C3 || trueecho
echo "=== done ==="--- `31-31`: **intelliLangModule OK — no leftover `intelliLangPlugin` usages found** gradle.properties contains `intelliLangModule=intellij.platform.langInjection` and a repo search shows no literal `intelliLangPlugin`; remaining `intelliLang` hits are plugin ids/resources and tests (e.g. plugin/intelliLang/src/main/resources/perl5.plugin.intelliLang.main.xml, plugin/intelliLang/build.gradle.kts, plugin/src/main/resources/META-INF/plugin.xml, plugin/src/test/resources/plugins/pluginsList.txt) and appear intentional. No further change required. </blockquote></details> <details> <summary>plugin/intelliLang/build.gradle.kts (1)</summary><blockquote> `35-35`: **Approve: bundledModules usage is correct.** gradle.properties — intelliLangModule=intellij.platform.langInjection (line 31) plugin/intelliLang/src/main/resources/perl5.plugin.intelliLang.main.xml — <module name="intellij.platform.langInjection"/> (line 21) </blockquote></details> <details> <summary>build.gradle.kts (1)</summary><blockquote> `369-371`: **LGTM — confirm intelliLangModule exists for CLion/PyCharm or gate it by IDE** intelliLangModule (gradle.properties = intellij.platform.langInjection) is added unconditionally to bundledModuleList (build.gradle.kts:369–413) and referenced in plugin/build.gradle.kts:31–32, plugin/intelliLang/build.gradle.kts:35, plugin/testFixtures/build.gradle.kts:36; if you run with CL/PC/PY, either verify those IDE distributions include langInjection or add a runWith-based guard to only bundle it for IDEs that ship the module. </blockquote></details> <details> <summary>plugin/testFixtures/build.gradle.kts (1)</summary><blockquote> `35-37`: **Test fixtures aligned with module bundling — verify module exists in Community** gradle.properties (line 31) sets intelliLangModule=intellij.platform.langInjection and plugin/testFixtures/build.gradle.kts bundles it via bundledModules(...). Verify the targeted Community platform (platformType/platformVersion in gradle.properties or intellijPlatform config) includes intellij.platform.langInjection; if not, add it explicitly to the test fixture's bundledModules or target a platform that includes it. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| myOriginal.myRoots = new LinkedHashMap<>(myRoots); | ||
| myOriginal.myModificationTracker++; | ||
| }, BuildableRootsChangeRescanningInfo.newInstance().addModule(myModule).buildInfo()) | ||
| }, false, true) |
There was a problem hiding this comment.
🧩 Analysis chain
Dropped targeted rescanning info; validate roots-change semantics/perf
Replacing BuildableRootsChangeRescanningInfo...addModule(myModule) with boolean flags may widen rescanning scope to the whole project. Confirm this still limits reindexing appropriately on 253; if a new API exists for module-scoped rescans, prefer it.
🏁 Script executed:
#!/bin/bash
# Inspect available makeRootsChange overloads and any RootsChangeRescanningInfo replacements in 253
rg -nP 'class\\s+ProjectRootManagerEx|makeRootsChange\\s*\\(' -g '!**/build/**' -C3
rg -n 'RootsChangeRescanningInfo|BuildableRootsChangeRescanningInfo' -C2Length of output: 5553
Restore module-scoped roots rescanning — avoid boolean overload that may trigger full-project reindex
PerlModuleExtension now calls ProjectRootManagerEx.makeRootsChange(..., false, true) (plugin/common/src/main/java/com/intellij/openapi/projectRoots/impl/PerlModuleExtension.java — call ~line 91) while BuildableRootsChangeRescanningInfo is still imported (line 32); the boolean overload does not convey module-scoped RescanningInfo and can widen rescanning to the whole project. Replace the boolean overload with BuildableRootsChangeRescanningInfo.addModule(myModule) or use the appropriate RootsChangeRescanningInfo API to limit reindexing to the module.
| private static final String ACCESS_TOKEN = "Bearer perm:YXV0b3JlcG9ydGVy.NjEtMjY=.FgsJOlHXSdCSQTUyfagUKU9WlkrLhV"; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Hardcoded YouTrack token committed — rotate immediately and externalize the secret
A permanent YouTrack token is embedded in source control. This is a credential leak (repo + forks + CI logs) and must be fixed. Revoke the exposed token(s) immediately, purge from history if feasible, and load tokens from a secure runtime source.
Hotfix (stop committing secrets right now):
- private static final String ACCESS_TOKEN = "Bearer <redacted>";
+ // HOTFIX: load at runtime; do not commit tokens
+ private static final String ACCESS_TOKEN = ADMIN_TOKEN;Recommended follow‑up (separate non-admin reporter token + guard when missing):
@@
- private static final String ADMIN_TOKEN = "Bearer " + YOUTRACK_PROPERTY_VALUE;
- private static final String ACCESS_TOKEN = "Bearer <redacted>";
+ private static final String ADMIN_TOKEN = "Bearer " + YOUTRACK_PROPERTY_VALUE;
+ @VisibleForTesting
+ public static final String REPORT_TOKEN_PROPERTY_KEY = "youtrack.report.token";
+ public static final String REPORT_TOKEN_VALUE = System.getProperty(REPORT_TOKEN_PROPERTY_KEY);
+ private static final String ACCESS_TOKEN = StringUtil.isEmpty(REPORT_TOKEN_VALUE) ? "" : "Bearer " + REPORT_TOKEN_VALUE;Optional correctness tidy‑up to avoid false positives in tests:
@@
@TestOnly
public static boolean hasAdminToken() {
- return StringUtil.isNotEmpty(ADMIN_TOKEN);
+ return StringUtil.isNotEmpty(YOUTRACK_PROPERTY_VALUE);
}Operational steps:
- Revoke both the newly committed token and any previously committed variants.
- Rotate new tokens; inject via JVM property (-Dyoutrack.report.token=...) or environment mapping in CI/IDE run configs; never commit.
- Consider history rewrite (git filter-repo/BFG) to remove the secret from the repo and forks if policy allows.
- Add secret scanning to CI to prevent reintroduction.
Verification (scan repo for other leaks):
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Hardcoded YouTrack-like tokens:"
rg -nP --no-heading -S '(Bearer\s+perm:|perm:[A-Za-z0-9._-]{16,})' -g '!**/build/**' -g '!**/out/**' -g '!**/dist/**' -g '!**/target/**'
echo
echo "Token-related constants/usages:"
rg -nP --no-heading -C2 'youtrack\.token|YOUTRACK.*TOKEN|ACCESS_TOKEN|ADMIN_TOKEN'Length of output: 180
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Attempting to show the reviewed file (plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java)..."
if [ -f plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java ]; then
echo "File found — printing lines 1..240 with numbers:"
nl -ba plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java | sed -n '1,240p'
else
echo "File not found at that path. Searching repository for YoutrackErrorHandler.java..."
rg -n --hidden -uu --no-ignore-vcs 'YoutrackErrorHandler' || true
echo
echo "Searching for files containing 'Youtrack' (no ignores):"
rg -n --hidden -uu --no-ignore-vcs 'Youtrack' || true
echo
echo "Full-text search for 'Bearer perm' and token-like constants (no ignores):"
rg -nP --hidden -uu -S '(Bearer\s+perm:|perm:[A-Za-z0-9._-]{16,})' || echo "no 'Bearer perm' matches"
echo
echo "Search for token-related identifiers (ACCESS_TOKEN, ADMIN_TOKEN, YOUTRACK_PROPERTY_VALUE) (no ignores):"
rg -nP --hidden -uu -S 'ACCESS_TOKEN|ADMIN_TOKEN|YOUTRACK_PROPERTY_VALUE|YOUTRACK.*TOKEN|youtrack' -C2 || echo "no matches"
fiLength of output: 208
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java"
echo "Checking file: $FILE"
if [ -f "$FILE" ]; then
echo "File exists. Printing lines 1..240:"
sed -n '1,240p' "$FILE" || true
else
echo "File not found: $FILE"
fi
echo
echo "Searching repo for YouTrack-like tokens (no ignore):"
rg -nP --hidden -uu -S '(Bearer\s+perm:|perm:[A-Za-z0-9._-]{16,})' || echo "no matches"
echo
echo "Searching repo for token-related identifiers (ACCESS_TOKEN|ADMIN_TOKEN|YOUTRACK|youtrack) (no ignore):"
rg -n --hidden -uu -S 'ACCESS_TOKEN|ADMIN_TOKEN|YOUTRACK|youtrack' -C2 || echo "no matches"Length of output: 27807
Revoke exposed YouTrack token and remove it from source
Verified: plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java — line 78 contains a hardcoded ACCESS_TOKEN.
- Revoke the exposed token in YouTrack and rotate affected credentials immediately.
- Remove the secret from the repository and (if allowed) purge history with git filter-repo or BFG.
- Replace the hardcoded value with a runtime-loaded secret (CI secret / env / JVM property) and make the code fail-safe when the token is missing.
- Add secret-scanning to CI to prevent reintroduction.
Hotfix (apply immediately):
- private static final String ACCESS_TOKEN = "Bearer <redacted>";
+ // HOTFIX: load at runtime; do not commit tokens
+ private static final String ACCESS_TOKEN = ADMIN_TOKEN;Recommended follow‑up (separate non-admin reporter token + guard when missing):
@@
- private static final String ADMIN_TOKEN = "Bearer " + YOUTRACK_PROPERTY_VALUE;
- private static final String ACCESS_TOKEN = "Bearer <redacted>";
+ private static final String ADMIN_TOKEN = "Bearer " + YOUTRACK_PROPERTY_VALUE;
+ @VisibleForTesting
+ public static final String REPORT_TOKEN_PROPERTY_KEY = "youtrack.report.token";
+ public static final String REPORT_TOKEN_VALUE = System.getProperty(REPORT_TOKEN_PROPERTY_KEY);
+ private static final String ACCESS_TOKEN = StringUtil.isEmpty(REPORT_TOKEN_VALUE) ? "" : "Bearer " + REPORT_TOKEN_VALUE;Optional test tidy‑up:
@@
@TestOnly
public static boolean hasAdminToken() {
- return StringUtil.isNotEmpty(ADMIN_TOKEN);
+ return StringUtil.isNotEmpty(YOUTRACK_PROPERTY_VALUE);
}📝 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.
| private static final String ACCESS_TOKEN = "Bearer perm:YXV0b3JlcG9ydGVy.NjEtMjY=.FgsJOlHXSdCSQTUyfagUKU9WlkrLhV"; | |
| // HOTFIX: load at runtime; do not commit tokens | |
| private static final String ACCESS_TOKEN = ADMIN_TOKEN; |
hurricup
force-pushed
the
dev/253
branch
3 times, most recently
from
bdf27f5 to
cbbe1e5
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (38)
plugin/berrybrew/build.gradle.kts (2)
29-31: Confirm intent to target Ultimate only; consider making edition configurable.Switching to IntelliJPlatformType.IntellijIdeaUltimate narrows compatibility (runIde/builds will require IU). If this module doesn’t rely on IU‑only APIs, prefer keeping Community to support both IDEs, or gate the edition behind a Gradle property for local dev/CI flexibility.
- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + val platformType = if (providers.gradleProperty("ideEdition").orNull == "IC") + IntelliJPlatformType.IntellijIdeaCommunity + else + IntelliJPlatformType.IntellijIdeaUltimate + create(platformType, platformVersionProvider.get()) { useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() }
30-31: Avoid hard failure when useInstaller property is unset.providers.gradleProperty("useInstaller").get() will throw if the property isn’t defined. Default to false to improve DX and CI robustness.
- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller").orNull?.toBoolean() ?: falseplugin/cpanminus/common/build.gradle.kts (1)
29-31: Switching to Ultimate narrows install base; make intent explicit and make property handling robust.
- If the plugin must remain installable on Community, this change will prevent that. Consider driving the product via a Gradle property and defaulting to IU only when explicitly requested.
providers.gradleProperty("useInstaller").get().toBoolean()will throw when the property is absent. Prefer a safe default using Provider mapping.Apply:
- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ - useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + // Allow selecting product via -PideaType=IU|IC (defaults to IU only if that's the new policy) + val ideaType = providers.gradleProperty("ideaType") + .map { if (it.equals("IU", true)) IntelliJPlatformType.IntellijIdeaUltimate else IntelliJPlatformType.IntellijIdeaCommunity } + .orElse(IntelliJPlatformType.IntellijIdeaUltimate) // set Community if you still support IC + .get() + create(ideaType, platformVersionProvider.get()) { + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).orElse(false).get() }Verification checklist:
- Confirm Marketplace target: IU-only or IC+IU? If IU-only, ensure plugin.xml/module deps reflect that and release notes call out the change.
- CI: Ensure caches/licenses are set to fetch IU artifacts and tests don’t assume IC-only plugins. I can provide a repo scan script to check for lingering
IntellijIdeaCommunityreferences and plugin.xml module dependencies on request.plugin/terminal/build.gradle.kts (3)
30-31: Make useInstaller resilient to missing property.get() will throw if the gradle property is absent. Provide a safe default.
- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).getOrElse(false)
33-34: Bundling Terminal may be redundant on IU; gate it by edition (optional).IU already includes org.jetbrains.plugins.terminal. Keeping it is harmless but noisy; if you parameterize the edition, only bundle it for IC.
- bundledPlugins("org.jetbrains.plugins.terminal") + if (platformType == IntelliJPlatformType.IntellijIdeaCommunity) { + bundledPlugins("org.jetbrains.plugins.terminal") + }
29-31: Parameterize IntelliJ edition (IC vs IU).Confirmed: IntellijIdeaUltimate is hard-coded across many build.gradle.kts files. Centralize an ideaEdition provider (default "IU") in the root and map it to IntelliJPlatformType when calling create(...) to avoid accidental IU‑only builds. Suggested change:
- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + // ideaEdition can be "IC" or "IU" (default IU) + val platformType = providers.gradleProperty("ideaEdition") + .orElse("IU") + .map { if (it.equals("IC", true)) IntelliJPlatformType.IntellijIdeaCommunity else IntelliJPlatformType.IntellijIdeaUltimate } + .get() + create(platformType, platformVersionProvider.get()){mason/framework/frontend/split/build.gradle.kts (2)
31-31: Avoid hard failure when the 'useInstaller' Gradle property is unset.
.get()throws if the property is absent. Prefer a safe default.Apply:
- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller").getOrElse("false").toBoolean()
29-32: Make platform type selectable via Gradle property (safer for local vs CI builds).Parameterizing the platform type lets you default to IU locally while allowing CI to opt into IC if needed.
Apply:
intellijPlatform { val platformVersionProvider: Provider<String> by rootProject.extra - create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ - useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + val platformType = providers.gradleProperty("platformType") + .map { if (it.equals("IC", ignoreCase = true)) IntelliJPlatformType.IntellijIdeaCommunity else IntelliJPlatformType.IntellijIdeaUltimate } + .getOrElse(IntelliJPlatformType.IntellijIdeaUltimate) + create(platformType, platformVersionProvider.get()){ + useInstaller = providers.gradleProperty("useInstaller").getOrElse("false").toBoolean() } }Usage examples:
- Default (IU): ./gradlew build
- Community: ./gradlew build -PplatformType=IC
plugin/makeMaker/build.gradle.kts (1)
29-31: Prefer lazy wiring for platform version (if the API allows).If create(...) accepts Provider, pass platformVersionProvider directly to improve configuration-cache friendliness and avoid premature resolution. Otherwise, consider validating with a clear error when the provider is empty.
plugin/moduleBuild/build.gradle.kts (2)
29-31: Switching to Ultimate narrows runtime compatibility — confirm intent and CI/dev implications.Targeting IntellijIdeaUltimate means the plugin won’t run on Community. Ensure this is intended and that:
- Plugin metadata (since/until build, product descriptors) aligns with IU-only.
- Plugin Verifier matrix is updated (IU only, or keep IC if dual‑targeted).
- Local/CI have licenses for runIde on IU.
29-29: Prefer lazy Provider for platform version (plugin supports Provider)
Use the Provider directly to avoid eager resolution — replace platformVersionProvider.get() with platformVersionProvider.
File: plugin/moduleBuild/build.gradle.kts — line 29create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider) { /* ... */ }plugin/perlbrew/build.gradle.kts (2)
28-31: Avoid eager Provider resolution for platform version.If
createaccepts Provider, pass the provider directly to keep configuration lazy.- val platformVersionProvider: Provider<String> by rootProject.extra - create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + val platformVersionProvider: Provider<String> by rootProject.extra + create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider) {If the API requires a String, keep
.get()as-is. Please confirm the plugin DSL version used in this project supports Provider here.
30-31: Provide a safe default for useInstaller to avoid property-missing failures.
.get()will throw if-PuseInstallerisn’t set. Map with a default.- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller") + .map { it.toBoolean() } + .orElse(false) + .get()mason/framework/common/build.gradle.kts (3)
28-28: Don’t hard‑fail when the Gradle property is missing; keep configuration cache friendlyget() will throw if useInstaller isn’t provided. Prefer a safe default and lazy mapping.
Apply:
- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller") + .map(String::toBoolean) + .getOrElse(false)
26-28: Preserve Provider laziness for platform version (if supported by your plugin version)If create(...) accepts Provider, pass the provider directly to avoid eager reads and improve configuration‑cache behavior. If not supported in your current plugin, ignore.
- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider){
27-29: Build-time optimization: cache larger IU distributionsIU downloads are heavy. Ensure Gradle caches are enabled and consider CI caching of the IDE distributions to avoid repeated downloads or disable useInstaller on CI when not needed.
plugin/cpan/build.gradle.kts (2)
30-30: Harden useInstaller property with a safe default.Current
.get()will fail if the property is unset. Map to Boolean with a default:- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers + .gradleProperty("useInstaller") + .map(String::toBoolean) + .orElse(false) + .get()
28-31: Avoid eager Provider.get() for platform version (if API supports Provider).Prefer passing the Provider to defer resolution; otherwise keep as is.
- val platformVersionProvider: Provider<String> by rootProject.extra - create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + val platformVersionProvider: Provider<String> by rootProject.extra + create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider){ useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).orElse(false).get() }Verify that
create(.., Provider<String>)is available in your applied IntelliJ Platform Gradle plugin version.mason/framework/frontend/build.gradle.kts (1)
28-30: Guard against missinguseInstallerGradle property to avoid configuration-time failures
.get()will throw if-PuseInstallerisn’t provided. Use a safe default and keep it lazy.Apply:
- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers + .gradleProperty("useInstaller") + .map { it.toBoolean() } + .orElse(false) + .get()plugin/frontend/split/src/main/java/com/jetbrains/rdclient/actions/base/PerlBackendDelegatingActionCustomization.kt (1)
27-27: Either honorfileTypeor explicitly suppress the unused parameterPerlFrontendUtil.isPluginDocument(...) exists and is used; no PerlFrontendUtil.isPluginFileType(FileType) helper was found. Choose one:
- Short-term (silence inspection):
+@Suppress("UNUSED_PARAMETER") override fun isAvailable(dataContext: DataContext, fileType: FileType?): Boolean = isAvailable(dataContext)
- Better (use fileType as a fallback when editor is absent — implement a helper if needed):
-override fun isAvailable(dataContext: DataContext, fileType: FileType?): Boolean = isAvailable(dataContext) +override fun isAvailable(dataContext: DataContext, fileType: FileType?): Boolean = + isAvailable(dataContext) || (fileType != null && PerlFrontendUtil.isPluginFileType(fileType)) // add isPluginFileType to PerlFrontendUtil if it doesn't existLocation: plugin/frontend/split/src/main/java/com/jetbrains/rdclient/actions/base/PerlBackendDelegatingActionCustomization.kt:27
mason/htmlmason/frontend/build.gradle.kts (1)
30-32: Switch to IU: confirm toolchain/compat targets and make useInstaller safe
- Looks good to target IntelliJPlatformType.IntellijIdeaUltimate here. Please ensure CI/plugin verifier matrices also include IU 253 targets (and IC only if you still intend compatibility).
- Minor:
.get()onuseInstallerwill fail if the property is missing. Prefer a safe default.- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).getOrElse(false)mojo/common/build.gradle.kts (1)
45-47: Consistent IU target; guard optional Gradle property
- IU change aligns with the PR direction. Verify build jobs and IDE run configs pick IU artifacts.
- Make
useInstallerresilient to missing gradle.properties entry.- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).getOrElse(false)mojo/frontend/build.gradle.kts (1)
28-30: IU targeting is fine; hardenuseInstaller
- Same optional guard for
useInstalleras elsewhere.- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).getOrElse(false)plugin/perlInstall/build.gradle.kts (1)
28-30: IU switch OK; keepuseInstalleroptional
- Apply the safe default pattern for
useInstaller.- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).getOrElse(false)embedded/frontend/build.gradle.kts (1)
26-30: Consistent with repo-wide Ultimate move.Change looks good; same note about a safe default for
useInstaller.- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller").orNull?.toBoolean() ?: falseembedded/backend/build.gradle.kts (1)
30-34: Ultimate target acknowledged; verify intended distribution.If Community builds are still needed for testing or publishing, consider a property-driven switch or CI matrix.
#!/bin/bash # Map modules now tied to Ultimate to plan CI cache/artifact changes rg -n 'create\(IntelliJPlatformType\.IntellijIdeaUltimate' -g '!**/build/**'mason/mason2/common/build.gradle.kts (1)
42-46: OK; minor resilience nit onuseInstaller.Safe default prevents local build failures when the property is absent.
- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller").orNull?.toBoolean() ?: falsemason/htmlmason/common/build.gradle.kts (1)
43-47: Ultimate switch LGTM.Repeat the safe-default tweak if you align across modules.
- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller").orNull?.toBoolean() ?: falseplugin/coverage/build.gradle.kts (1)
29-35: Coverage: remove duplicate bundling of the coverage plugin and the agent or document why both are required.gradle.properties defines coveragePlugin=Coverage and both the plugin and intellij.platform.coverage.agent are bundled in multiple places:
- gradle.properties:33 — coveragePlugin=Coverage
- plugin/coverage/build.gradle.kts:33-34 — bundledPlugins(...), bundledModule("intellij.platform.coverage.agent")
- plugin/build.gradle.kts:36,92 — providers.gradleProperty("coveragePlugin").get(), "intellij.platform.coverage.agent"
- build.gradle.kts:391 — bundledPluginList.add(providers.gradleProperty("coveragePlugin").get())
If the agent module suffices, remove the extra plugin; otherwise keep both and add a short comment explaining why both are needed.
plugin/idea/build.gradle.kts (1)
29-34: Remove redundant com.intellij.java bundling for IntelliJ UltimateJava is already included in Ultimate; drop or gate this call to avoid resolution quirks.
File: plugin/idea/build.gradle.kts:33- bundledPlugins("com.intellij.java") + // On Community builds you may need this. On Ultimate it's redundant. + // bundledPlugins("com.intellij.java")embedded/frontend/split/build.gradle.kts (1)
29-32: Consistent IU migration; apply safe default for useInstaller here tooSame suggestion as other modules to avoid configuration-time failures when
-PuseInstalleris absent.- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ - useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).orElse(false).get()Confirm tests still run green under IU downloads in CI (larger IDE footprint).
tt2/frontend/split/build.gradle.kts (1)
29-32: IU upgrade acknowledged; mirror the resilient property handling- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ - useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).orElse(false).get()mojo/frontend/split/build.gradle.kts (1)
28-32: Good with IU; align property handling with other modules- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ - useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).orElse(false).get()mason/htmlmason/frontend/split/build.gradle.kts (1)
31-35: IU switch aligns with framework modules; adopt safer gradleProperty usage- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ - useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).orElse(false).get()Confirm any Ultimate‑only dependencies are declared in plugin descriptors where needed (e.g., added module dependencies).
plugin/frontend/split/build.gradle.kts (1)
26-34: IU target + bundled modules: verify module availability and names
bundledModules("intellij.rd.ide.model.generated")must exist in the chosen IU version; module names are case‑sensitive. KeepbundledLibrary(...)path stable across IDE versions.Please run a local
prepareSandboxandverifyPluginto ensure packaging succeeds.Also harden
useInstaller:- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ - useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).orElse(false).get()tt2/common/build.gradle.kts (1)
42-46: IU migration OK; CI/cache considerationsLarge IU downloads can affect CI time/storage. Ensure shared caches are enabled (Gradle + IDE artifact cache).
Enable Gradle build cache and persist the JetBrains IDE artifacts directory across CI jobs to reduce fetch time.
For consistency with other modules:
- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ - useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).orElse(false).get()mason/mason2/frontend/build.gradle.kts (1)
29-32: Ultimate switch OK — harden useInstaller provider accessroot gradle.properties defines
useInstaller=false(gradle.properties:27), so missing-prop failures are unlikely, but make the access provider-safe.- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ - useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).orElse(false).get()Checks performed: no
IntellijIdeaCommunityreferences found; could not locateidea-version/since-build/until-buildentries in plugin.xml — verify plugin.xml ranges and CI compatibility with the selected IU version.tt2/frontend/build.gradle.kts (1)
29-29: MakeuseInstallerproperty safe-by-default
.get()will fail if the Gradle property isn’t defined. Prefer a lazy default:- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller") + .map { it.toBoolean() } + .orElse(false) + .get()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
embedded/backend/build.gradle.kts(1 hunks)embedded/build.gradle.kts(1 hunks)embedded/common/build.gradle.kts(1 hunks)embedded/frontend/build.gradle.kts(1 hunks)embedded/frontend/split/build.gradle.kts(1 hunks)gradle.properties(1 hunks)mason/framework/common/build.gradle.kts(1 hunks)mason/framework/frontend/build.gradle.kts(1 hunks)mason/framework/frontend/split/build.gradle.kts(1 hunks)mason/htmlmason/common/build.gradle.kts(1 hunks)mason/htmlmason/frontend/build.gradle.kts(1 hunks)mason/htmlmason/frontend/split/build.gradle.kts(1 hunks)mason/mason2/common/build.gradle.kts(1 hunks)mason/mason2/frontend/build.gradle.kts(1 hunks)mason/mason2/frontend/split/build.gradle.kts(1 hunks)mojo/common/build.gradle.kts(1 hunks)mojo/frontend/build.gradle.kts(1 hunks)mojo/frontend/split/build.gradle.kts(1 hunks)plugin/asdf/build.gradle.kts(1 hunks)plugin/berrybrew/build.gradle.kts(1 hunks)plugin/carton/build.gradle.kts(1 hunks)plugin/common/build.gradle.kts(1 hunks)plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java(2 hunks)plugin/coverage/build.gradle.kts(1 hunks)plugin/cpan/build.gradle.kts(1 hunks)plugin/cpanminus/backend/build.gradle.kts(1 hunks)plugin/cpanminus/common/build.gradle.kts(1 hunks)plugin/debugger/build.gradle.kts(1 hunks)plugin/frontend/build.gradle.kts(1 hunks)plugin/frontend/split/build.gradle.kts(1 hunks)plugin/frontend/split/src/main/java/com/jetbrains/rdclient/actions/base/PerlBackendDelegatingActionCustomization.kt(1 hunks)plugin/frontend/split/src/main/java/com/jetbrains/rdclient/editorActions/cwm/PerlFrontendEditorActionHandlerStrategyCustomizer.kt(2 hunks)plugin/idea/build.gradle.kts(1 hunks)plugin/intelliLang/build.gradle.kts(1 hunks)plugin/makeMaker/build.gradle.kts(1 hunks)plugin/moduleBuild/build.gradle.kts(1 hunks)plugin/perlInstall/build.gradle.kts(1 hunks)plugin/perlbrew/build.gradle.kts(1 hunks)plugin/plenv/build.gradle.kts(1 hunks)plugin/terminal/build.gradle.kts(1 hunks)plugin/testFixtures/build.gradle.kts(1 hunks)tt2/common/build.gradle.kts(1 hunks)tt2/frontend/build.gradle.kts(1 hunks)tt2/frontend/split/build.gradle.kts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- plugin/testFixtures/build.gradle.kts
- plugin/frontend/split/src/main/java/com/jetbrains/rdclient/editorActions/cwm/PerlFrontendEditorActionHandlerStrategyCustomizer.kt
- plugin/intelliLang/build.gradle.kts
- gradle.properties
- plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java
⏰ 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). (3)
- GitHub Check: Build (macos-latest)
- GitHub Check: Build (ubuntu-latest)
- GitHub Check: Build (windows-latest)
🔇 Additional comments (20)
plugin/common/build.gradle.kts (1)
39-41: Ultimate migration — confirm IU-only targeting, product descriptors, and CI readiness
- Repo scan: all intellijPlatform.create(...) entries use IntellijIdeaUltimate; no IntellijIdeaCommunity matches (change at plugin/common/build.gradle.kts:39–41 is consistent repo-wide).
- The plugin.xml since/until-build bounds were not found by the provided search — verify META-INF/plugin.xml and any Marketplace/product metadata include correct product targeting and since/until-build (and productCode) for IU.
- useInstaller is read from a Gradle property in many modules (providers.gradleProperty("useInstaller").get().toBoolean()); if you enable useInstaller=true, update CI download/caching and packaging steps to handle larger IU installers and installer-format differences.
mason/framework/frontend/split/build.gradle.kts (1)
30-30: Switch to Ultimate (IU) acknowledged — no Community (IC) usages found.Search output contains only IntellijIdeaUltimate across the repo; confirm CI workflows (.github/workflows) are configured to download/cache the Ultimate IDE (IU) installers (not IC).
plugin/makeMaker/build.gradle.kts (1)
29-29: IU platform switch: confirm intentional product targeting and downstream metadata.Changing to IntelliJPlatformType.IntellijIdeaUltimate narrows compatibility to IU-based IDEs. Please confirm CE support isn’t required for makeMaker, and ensure Marketplace descriptors (product compatibility), plugin verifier matrices, and since/until-builds are updated accordingly across modules/CI.
plugin/debugger/build.gradle.kts (1)
29-31: Ultimate SDK implies IU-only or risk of IC runtime breakage — confirm intent and gate/build accordingly.plugin/debugger/build.gradle.kts (lines 29–31) now creates IntellijIdeaUltimate. Choose one:
- Keep IC compatibility — build both Community and Ultimate variants and run the Plugin Verifier against IC. Suggested change (add Community target before Ultimate):
val platformVersionProvider: Provider<String> by rootProject.extra - create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + // Build IC and IU variants to ensure compatibility across editions + create(IntelliJPlatformType.IntellijIdeaCommunity, platformVersionProvider.get()) + create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() }
- Drop IC support — add a hard dependency in plugin.xml to prevent installation on Community:
<depends>com.intellij.modules.ultimate</depends>Automated verification failed (ripgrep: "No files were searched"). Run locally and share results or confirm changes:
fd -a -i 'plugin*.xml' | xargs -I{} rg -nH -C2 -P 'com.intellij.modules.ultimate' {}
rg -nP --type=gradle -C2 'IntelliJPlatformType.IntellijIdea(Community|Ultimate)'
rg -nP -C2 'com.intellij..*ultimate' -g '!/build/'plugin/moduleBuild/build.gradle.kts (1)
29-31: Require org.jetbrains.intellij.platform >= v2.0.0-beta9The intellijPlatform { create(...) } DSL was introduced in org.jetbrains.intellij.platform v2.0.0-beta9 — update/check the root plugin version (pluginManagement/plugins or build.gradle[.kts]) and ensure all modules use a compatible version to avoid method-missing errors.
plugin/cpanminus/backend/build.gradle.kts (1)
31-33: Switch to Ultimate: make edition configurable; verify plugin.xml and CIFile: plugin/cpanminus/backend/build.gradle.kts (lines 31–33)
- Building against IntelliJ IDEA Ultimate restricts contributors and increases CI artifact downloads; gate the edition with a Gradle property so local/CI can use IC while releases use IU.
- Check plugin.xml for com.intellij.modules.ultimate and remove unless strictly required; confirm CI agents can fetch IU artifacts and caches won’t break across edition changes.
- Suggested tweak (keep IU default, allow IC via -PijEdition=IC; harden useInstaller parsing):
- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ - useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + val edition = providers.gradleProperty("ijEdition").orElse("IU").get() + val platformType = if (edition.equals("IU", ignoreCase = true)) + IntelliJPlatformType.IntellijIdeaUltimate + else + IntelliJPlatformType.IntellijIdeaCommunity + create(platformType, platformVersionProvider.get()) { + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).orElse(false).get() }
- Verification: repo search didn’t run here (ripgrep returned no files). Run these locally to confirm no Ultimate-only deps or edition drift:
rg -nP 'com\.intellij\.modules\.ultimate' -C2 rg -nP 'IntelliJPlatformType\.IntellijIdea(Community|Ultimate)' -C2 rg -nP 'bundledPlugin\(|plugins\s*\{' -C3plugin/perlbrew/build.gradle.kts (1)
29-29: Switch to IntelliJ IDEA Ultimate is repo-wide — confirm intent & Marketplace/product impact.
- Search found IntellijIdeaUltimate used in many modules (e.g. plugin/perlbrew/build.gradle.kts:29) and a root mapping to IntelliJIdeaUltimate (build.gradle.kts ~393–394); publishPlugin is configured (build.gradle.kts publishPlugin block ~241–243).
- Action: if you intend IU‑only, ensure plugin.xml(s) and the JetBrains Marketplace listing declare IU‑only support; if you need IC support, revert this module to IntellijIdeaCommunity.
mason/framework/common/build.gradle.kts (1)
27-29: Ultimate platform migration — verify product compatibility & CI/runIde implicationsI ran the check you provided: ripgrep returned "No files were searched" (likely ignore filters) and fd found multiple plugin.xml files but the previous run produced no or matches, so I can't confirm whether Community targets remain or whether any descriptors require IU-only modules. Confirm intent and CI coverage.
Modules with plugin.xml found:
- embedded/src/main/resources/META-INF/plugin.xml
- mason/framework/src/main/resources/META-INF/plugin.xml
- mason/htmlmason/src/main/resources/META-INF/plugin.xml
- mason/mason2/src/main/resources/META-INF/plugin.xml
- mojo/src/main/resources/META-INF/plugin.xml
- plugin/src/main/resources/META-INF/plugin.xml
- tt2/src/main/resources/META-INF/plugin.xml
Action items (short):
- Confirm the switch to Intellij IDEA Ultimate is intentional across the repo.
- Verify CI/runners have IU licenses for runIde / UI tests, or update CI to skip/adjust those jobs.
- Inspect each plugin.xml for IU-only entries or constraints and update documentation or descriptors.
Suggested verification script to run locally (copy/paste and paste results back):
#!/bin/bash set -euo pipefail echo "Searching for IntellijIdeaCommunity occurrences..." rg -nP --hidden --no-ignore '\bIntellijIdeaCommunity\b' || true echo -e "\nSearching for IntellijIdeaUltimate / IntellijPlatformType.IntellijIdeaUltimate..." rg -nP --hidden --no-ignore 'IntellijIdeaUltimate' || true rg -nP --hidden --no-ignore 'IntellijPlatformType\.IntellijIdeaUltimate' || true echo -e "\nSearching for gradle intellij plugin / runIde usages (CI/local)..." rg -nP --hidden --no-ignore '\bintellij\s*\{' || true rg -nP --hidden --no-ignore '\brunIde\b' || true echo -e "\nPlugin descriptors and 'idea-version' / 'depends' lines (or first 80 lines if none):" while IFS= read -r f; do echo "== ${f} ==" rg -n -e '<idea-version[^>]*>' -e '<depends[^>]*>' "$f" || sed -n '1,80p' "$f" done < <(fd -HI -e xml -t f)plugin/cpan/build.gradle.kts (1)
29-31: Switching to Ultimate (IU) has build/licensing and compatibility implications—confirm intent.
- IU artifacts require authenticated downloads in CI; ensure credentials/tokens are in place.
- If you still intend Community (IC) compatibility for the plugin, run Plugin Verifier against IC builds to avoid accidental IU API usage.
Run this read‑only repo scan to sanity‑check platform usage and property wiring:
mason/framework/frontend/build.gradle.kts (1)
28-30: Ultimate platform switch — verified repository-wideVerified: all intellijPlatform.create(...) in .gradle.kts set IntellijIdeaUltimate; no IntellijIdeaCommunity/ideaIC/IC references or plugin.xml since-build/until-build entries found — change is intentional and consistent.
plugin/frontend/split/src/main/java/com/jetbrains/rdclient/actions/base/PerlBackendDelegatingActionCustomization.kt (2)
21-21: Import aligns with the new API signature — OK.Adding
FileTypeis required for the updated override; no issues.
29-30: Null-safety ofisPluginDocument— wrap inlet-private fun isAvailable(dataContext: DataContext): Boolean = Registry.`is`("perl5.frontend.speculative.actions.enabled") && - PerlFrontendUtil.isPluginDocument(EDITOR.getData(dataContext)?.document) +private fun isAvailable(dataContext: DataContext): Boolean = + Registry.`is`("perl5.frontend.speculative.actions.enabled") && + (EDITOR.getData(dataContext)?.document?.let { PerlFrontendUtil.isPluginDocument(it) } == true)Unable to locate the
isPluginDocumentsignature to confirm its nullability—please verify and ensure this change preserves the original behavior.plugin/carton/build.gradle.kts (1)
29-31: IU platform switch acknowledged; compatibility check
- Moving compile/runtime against IU can accidentally pull IU‑only APIs. If IC support is still desired, ensure plugin.xml/module deps avoid IU‑only modules and run verifier against IC too.
embedded/common/build.gradle.kts (1)
42-44: Aligned with IU; verify generated sources workflow
- IU change LGTM. Double‑check any codegen tasks still resolve against 253 IU SDKs on CI agents.
plugin/frontend/build.gradle.kts (1)
27-29: IU move consistent; check IntelliLang/module deps after bundling change
- Given the PR’s IntelliLang plugin→module shift, confirm required modules (e.g., langInjection) are present under IU 253 and no leftover bundledPlugins remain.
embedded/build.gradle.kts (1)
39-41: IU target confirmed; packaging block unchanged
- Platform switch is fine. Ensure
pluginModule(...)content still matches IU artifacts and that the local:pluginis verified against IU 253.plugin/plenv/build.gradle.kts (1)
28-32: Ultimate migration — approved; useInstaller present
- LGTM switching to IntellijIdeaUltimate — no IntellijIdeaCommunity occurrences found.
- providers.gradleProperty("useInstaller").get() is safe: gradle.properties defines useInstaller=false.
mason/mason2/frontend/split/build.gradle.kts (1)
31-35: Ultimate migration consistent; OK.
No functional concerns — verified mason/mason2/frontend/split/build.gradle.kts and other split frontends consistently create IntelliJPlatformType.IntellijIdeaUltimate and set useInstaller from the gradle property (gradle.properties: useInstaller=false).plugin/asdf/build.gradle.kts (1)
28-32: IU target OK; double-check runtime/test matrix for IU-only modulesVerification incomplete — rg returned "No files were searched" (negative-only glob), so cannot confirm absence of IntellijIdeaCommunity references.
Location: plugin/asdf/build.gradle.kts (lines 28–32)
Re-run local check:
rg -nP --ignore-dir build 'IntellijIdeaCommunity'Optional refactor:
- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ - useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).orElse(false).get()tt2/frontend/build.gradle.kts (1)
28-30: Edition switch to Ultimate: verify intentional IU-only targeting and downstream configsChanging to
IntelliJPlatformType.IntellijIdeaUltimatenarrows compatibility to IU builds. Please confirm:
- Marketplace/product matrix and plugin verifier targets are updated to IU.
- runIde/sandbox and CI cache use IU artifacts.
- Any modules still expecting IC are reconciled.
If this is the planned direction for the whole repo, LGTM once verified.
| create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ | ||
| useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() | ||
| } |
There was a problem hiding this comment.
Guard against missing Gradle property to avoid build failure.
providers.gradleProperty("useInstaller").get() throws if the property is absent. Default to false safely.
Apply this diff:
- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){
- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean()
+ create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()) {
+ useInstaller = providers.gradleProperty("useInstaller").orNull?.toBoolean() ?: 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.
| create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ | |
| useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() | |
| } | |
| create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()) { | |
| useInstaller = providers.gradleProperty("useInstaller").orNull?.toBoolean() ?: false | |
| } |
🤖 Prompt for AI Agents
In plugin/common/build.gradle.kts around lines 39 to 41, the call to
providers.gradleProperty("useInstaller").get() will throw if the property is
absent; change it to safely default to false by reading the property as nullable
and falling back to "false" (e.g. use
providers.gradleProperty("useInstaller").orNull?.toBoolean() ?: false) so the
build doesn't fail when the property is missing.
| useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() | ||
| } |
There was a problem hiding this comment.
Guard against missing Gradle property to avoid configuration-time failures.
providers.gradleProperty("useInstaller").get() throws if the property is absent. Default this to false to keep local/CI builds resilient.
Apply this diff:
- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean()
+ useInstaller = providers.gradleProperty("useInstaller").orNull?.toBoolean() ?: 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.
| useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() | |
| } | |
| useInstaller = providers.gradleProperty("useInstaller").orNull?.toBoolean() ?: false | |
| } |
🤖 Prompt for AI Agents
plugin/makeMaker/build.gradle.kts lines 30-31: the call
providers.gradleProperty("useInstaller").get().toBoolean() throws if the
property is missing; change it to default to false by invoking orElse("false")
on the provider before calling get() and toBoolean() (i.e., use
providers.gradleProperty("useInstaller").orElse("false").get().toBoolean()) so
missing property yields false instead of failing configuration.
| val platformVersionProvider: Provider<String> by rootProject.extra | ||
| create(IntelliJPlatformType.IntellijIdeaCommunity, platformVersionProvider.get()){ | ||
| create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ | ||
| useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() |
There was a problem hiding this comment.
Avoid throwing when the Gradle property is missing.
providers.gradleProperty("useInstaller").get() will fail if unset. Default to false safely.
Apply this diff:
- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean()
+ useInstaller = providers.gradleProperty("useInstaller").orNull?.toBoolean() ?: 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.
| useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() | |
| useInstaller = providers.gradleProperty("useInstaller").orNull?.toBoolean() ?: false |
🤖 Prompt for AI Agents
In plugin/moduleBuild/build.gradle.kts around line 30, the current code calls
providers.gradleProperty("useInstaller").get().toBoolean() which throws if the
Gradle property is not set; change it to safely handle a missing property by
using providers.gradleProperty("useInstaller").orNull?.toBoolean() ?: false so
that when the property is absent it defaults to false instead of throwing.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (18)
plugin/debugger/build.gradle.kts (2)
29-29: Ultimate-only target: confirm intended Marketplace compatibility.Switching to IntelliJPlatformType.IntellijIdeaUltimate makes this module (and likely the plugin) installable only on IntelliJ IDEA Ultimate. If Community support is still desired, keep Community or make the edition configurable via a Gradle property.
Suggested configurable edition:
- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + val platformType = + if (providers.gradleProperty("ideaEdition").orElse("IC").get() == "IU") + IntelliJPlatformType.IntellijIdeaUltimate + else + IntelliJPlatformType.IntellijIdeaCommunity + create(platformType, platformVersionProvider.get()) {
30-31: Guard against missing Gradle property for useInstaller.providers.gradleProperty("useInstaller").get() throws if the property is absent. Default it to false to avoid configuration-time failures.
Apply:
- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller").orElse("false").get().toBoolean()embedded/common/build.gradle.kts (1)
42-44: Switch to Ultimate: verify target platform and harden property handling
Confirm this module is intended to be IU-only. If CE compatibility is still desired elsewhere, consider a matrix (CE+IU) or a property‑driven platform type to avoid silently dropping CE users. Also ensure plugin.xml/module deps and Marketplace channel metadata align with IU.
providers.gradleProperty("useInstaller").get()throws when the prop is absent. Provide a safe default to keep CI/dev builds reproducible.Apply:
- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ - useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()) { + useInstaller = providers.gradleProperty("useInstaller") + .map(String::toBoolean) + .orElse(false) + .get() }Optional: add a property to toggle CE/IU without code changes:
val ijType = providers.gradleProperty("ij.type").orElse("IU").get() create( if (ijType.equals("CE", true)) IntelliJPlatformType.IntellijIdeaCommunity else IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get() ) { /* ... */ }Please also run Plugin Verifier against IU (and CE if dual‑targeted) in CI to catch regressions.
plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java (4)
229-232: hasAdminToken() can return true even when token is missingADMIN_TOKEN is always non-empty ("Bearer " prefix). Check the property instead.
- return StringUtil.isNotEmpty(ADMIN_TOKEN); + return StringUtil.isNotEmpty(YOUTRACK_PROPERTY_VALUE);
2-2: Copyright year looks backdatedProject year is 2025; avoid regressing the header year.
- * Copyright 2015-2024 Alexandr Evstigneev + * Copyright 2015-2025 Alexandr Evstigneev
200-207: Treat any 2xx as success, not only 200YouTrack may return other 2xx codes. Current check will log false negatives.
- if (statusLine.getStatusCode() != 200) { + int sc = statusLine.getStatusCode(); + if (sc < 200 || sc >= 300) { LOGGER.warn("Error attaching files: status=" + statusLine +- if (statusLine.getStatusCode() != 200) { + int sc = statusLine.getStatusCode(); + if (sc < 200 || sc >= 300) { LOGGER.warn("Error submitting report: status=" + statusLine +Also applies to: 262-267
186-194: HTTP client has no timeouts — risk of indefinite hangsApache HttpClient defaults to no timeouts; external calls can block background tasks indefinitely.
var requestConfig = org.apache.http.client.config.RequestConfig.custom() .setConnectTimeout(10_000) .setConnectionRequestTimeout(5_000) .setSocketTimeout(20_000) .build(); try (CloseableHttpClient httpClient = HttpClients.custom().setDefaultRequestConfig(requestConfig).build()) { // ... }Also applies to: 235-254
plugin/testFixtures/src/testFixtures/java/base/PerlDistributionTestCase.kt (3)
22-27: Narrow the @OptIn scope (optional).Limit IntellijInternalApi opt‑in to the smallest scope to reduce blast radius in tests.
-@OptIn(IntellijInternalApi::class) open class PerlDistributionTestCase : PerlLightTestCaseBase() { @@ - fun testDistribution(pluginId: String) { + @OptIn(IntellijInternalApi::class) + fun testDistribution(pluginId: String) {
45-49: Avoid NPEs and flakiness: don’t use!!and sort output.Use
orEmpty()and sort paths for deterministic test output.- " - ${moduleDescriptor.moduleId} (${moduleDescriptor.moduleLoadingRule}); ${ - moduleDescriptor.jarFiles!!.joinToString { file -> relativizer(file) } - }\n" + " - ${moduleDescriptor.moduleId} (${moduleDescriptor.moduleLoadingRule}); ${ + moduleDescriptor.jarFiles.orEmpty().map(relativizer).sorted().joinToString() + }\n"
40-42: Same here: remove!!and sort jar list.Mirrors the change above for consistency and stable snapshots.
- result.append("Jar files:\n") - pluginDescriptor.jarFiles!!.forEach { result.append("- ${relativizer(it)}\n") } + result.append("Jar files:\n") + pluginDescriptor.jarFiles.orEmpty() + .map(relativizer) + .sorted() + .forEach { result.append("- $it\n") }mason/mason2/frontend/build.gradle.kts (2)
31-31: Make gradleProperty access resilient.
.get()will throw if-PuseInstalleris absent. Prefer a safe default.Apply:
- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller") + .map(String::toBoolean) + .getOrElse(false)
29-32: Parameterize the platform edition via a Gradle property.Keeps local/CI flexibility (e.g.,
-PideaPlatformType=IC|IU) while defaulting to IU for your migration.Apply:
intellijPlatform { val platformVersionProvider: Provider<String> by rootProject.extra - create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ - useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + val platformType = providers.gradleProperty("ideaPlatformType") + .map { it.lowercase() } + .map { + when (it) { + "iu", "ultimate" -> IntelliJPlatformType.IntellijIdeaUltimate + "ic", "community" -> IntelliJPlatformType.IntellijIdeaCommunity + else -> IntelliJPlatformType.IntellijIdeaUltimate + } + } + .getOrElse(IntelliJPlatformType.IntellijIdeaUltimate) + create(platformType, platformVersionProvider.get()){ + useInstaller = providers.gradleProperty("useInstaller") + .map(String::toBoolean) + .getOrElse(false) } }mason/framework/common/build.gradle.kts (2)
26-28: Consider avoiding eager .get() on platformVersionProvider.If the
createAPI accepts a Provider, pass the provider directly to keep configuration lazy. If not, keep as is.Try this and revert if the API requires String:
- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider) {
28-29: Prefer lazy provider mapping foruseInstallerinstead of .get().gradle.properties:27 contains
useInstaller=falseso the current .get() won't throw, but change to a lazy provider with a safe default to avoid eager reads or future missing-property failures.- useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + useInstaller = providers.gradleProperty("useInstaller") + .map(String::toBoolean) + .getOrElse(false)plugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.kt (2)
30-31: Deduplicate and validate watch roots; tighten RA genericAvoid duplicate watches and invalid files; returning Collection aligns with base API.
Apply:
- override fun getRootsToWatch(project: Project): List<VirtualFile> = - PerlProjectManager.getInstance(project).allLibraryRoots + ReadAction.compute<List<VirtualFile>, Throwable> { PerlRunUtil.getBinDirectories(project).toList() } + override fun getRootsToWatch(project: Project): Collection<VirtualFile> = + (PerlProjectManager.getInstance(project).allLibraryRoots.asSequence() + + ReadAction.compute<List<VirtualFile>, RuntimeException> { + PerlRunUtil.getBinDirectories(project).toList() + }.asSequence()) + .filter(VirtualFile::isValid) + .distinct() + .toList()
2-2: Update copyright yearNew file in 2025; keep the header current.
- * Copyright 2015-2023 Alexandr Evstigneev + * Copyright 2015-2025 Alexandr Evstigneevmojo/src/test/resources/unit/distribution/plugin.none.txt (1)
14-15: Dependency ordering: confirm determinism.The order (
perl5.plugin.backend.mainbeforeperl5.lang.mojo.common.main) isn’t alphabetical; if order is topological, please ensure the generator enforces it so this fixture won’t flap across JDK/OS/iterator variations.If alphabetical is acceptable, consider sorting for stability. Otherwise, add a brief comment in the generator describing the intended order.
plugin/src/test/resources/unit/distribution/plugin.none.txt (1)
8-97: Stabilize dependency listing order in generatorFor golden files, ensure deps are emitted in a deterministic order (e.g., lexicographic) to prevent spurious diffs when inputs reorder. If already sorted, ignore.
Would you like a small patch to sort dependencies before rendering?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (56)
build.gradle.kts(2 hunks)embedded/backend/build.gradle.kts(1 hunks)embedded/build.gradle.kts(1 hunks)embedded/common/build.gradle.kts(1 hunks)embedded/frontend/build.gradle.kts(1 hunks)embedded/frontend/split/build.gradle.kts(1 hunks)embedded/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)gradle.properties(1 hunks)mason/framework/common/build.gradle.kts(1 hunks)mason/framework/frontend/build.gradle.kts(1 hunks)mason/framework/frontend/split/build.gradle.kts(1 hunks)mason/framework/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)mason/htmlmason/common/build.gradle.kts(1 hunks)mason/htmlmason/frontend/build.gradle.kts(1 hunks)mason/htmlmason/frontend/split/build.gradle.kts(1 hunks)mason/htmlmason/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)mason/mason2/common/build.gradle.kts(1 hunks)mason/mason2/frontend/build.gradle.kts(1 hunks)mason/mason2/frontend/split/build.gradle.kts(1 hunks)mason/mason2/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)mojo/common/build.gradle.kts(1 hunks)mojo/frontend/build.gradle.kts(1 hunks)mojo/frontend/split/build.gradle.kts(1 hunks)mojo/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)plugin/asdf/build.gradle.kts(1 hunks)plugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.java(0 hunks)plugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.kt(1 hunks)plugin/berrybrew/build.gradle.kts(1 hunks)plugin/build.gradle.kts(1 hunks)plugin/carton/build.gradle.kts(1 hunks)plugin/common/build.gradle.kts(1 hunks)plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java(2 hunks)plugin/coverage/build.gradle.kts(1 hunks)plugin/cpan/build.gradle.kts(1 hunks)plugin/cpanminus/backend/build.gradle.kts(1 hunks)plugin/cpanminus/common/build.gradle.kts(1 hunks)plugin/debugger/build.gradle.kts(1 hunks)plugin/frontend/build.gradle.kts(1 hunks)plugin/frontend/split/build.gradle.kts(1 hunks)plugin/frontend/split/src/main/java/com/jetbrains/rdclient/actions/base/PerlBackendDelegatingActionCustomization.kt(1 hunks)plugin/frontend/split/src/main/java/com/jetbrains/rdclient/editorActions/cwm/PerlFrontendEditorActionHandlerStrategyCustomizer.kt(2 hunks)plugin/idea/build.gradle.kts(1 hunks)plugin/intelliLang/build.gradle.kts(1 hunks)plugin/makeMaker/build.gradle.kts(1 hunks)plugin/moduleBuild/build.gradle.kts(1 hunks)plugin/perlInstall/build.gradle.kts(1 hunks)plugin/perlbrew/build.gradle.kts(1 hunks)plugin/plenv/build.gradle.kts(1 hunks)plugin/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)plugin/terminal/build.gradle.kts(1 hunks)plugin/testFixtures/build.gradle.kts(1 hunks)plugin/testFixtures/src/testFixtures/java/base/PerlDistributionTestCase.kt(2 hunks)tt2/common/build.gradle.kts(1 hunks)tt2/frontend/build.gradle.kts(1 hunks)tt2/frontend/split/build.gradle.kts(1 hunks)tt2/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)
💤 Files with no reviewable changes (1)
- plugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.java
✅ Files skipped from review due to trivial changes (1)
- mason/framework/src/test/resources/unit/distribution/plugin.none.txt
🚧 Files skipped from review as they are similar to previous changes (35)
- mason/framework/frontend/split/build.gradle.kts
- plugin/coverage/build.gradle.kts
- mojo/frontend/build.gradle.kts
- tt2/frontend/build.gradle.kts
- mason/htmlmason/frontend/split/build.gradle.kts
- plugin/cpanminus/backend/build.gradle.kts
- embedded/build.gradle.kts
- plugin/moduleBuild/build.gradle.kts
- plugin/frontend/split/build.gradle.kts
- plugin/intelliLang/build.gradle.kts
- plugin/plenv/build.gradle.kts
- tt2/common/build.gradle.kts
- mojo/common/build.gradle.kts
- plugin/makeMaker/build.gradle.kts
- plugin/frontend/split/src/main/java/com/jetbrains/rdclient/editorActions/cwm/PerlFrontendEditorActionHandlerStrategyCustomizer.kt
- embedded/frontend/build.gradle.kts
- gradle.properties
- mason/framework/frontend/build.gradle.kts
- plugin/testFixtures/build.gradle.kts
- tt2/frontend/split/build.gradle.kts
- embedded/frontend/split/build.gradle.kts
- mason/mason2/frontend/split/build.gradle.kts
- mason/htmlmason/common/build.gradle.kts
- plugin/terminal/build.gradle.kts
- build.gradle.kts
- plugin/cpan/build.gradle.kts
- mason/htmlmason/frontend/build.gradle.kts
- plugin/perlbrew/build.gradle.kts
- plugin/carton/build.gradle.kts
- embedded/backend/build.gradle.kts
- plugin/frontend/build.gradle.kts
- plugin/common/build.gradle.kts
- mojo/frontend/split/build.gradle.kts
- plugin/berrybrew/build.gradle.kts
- plugin/idea/build.gradle.kts
⏰ 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). (3)
- GitHub Check: Build (ubuntu-latest)
- GitHub Check: Build (macos-latest)
- GitHub Check: Build (windows-latest)
🔇 Additional comments (32)
plugin/asdf/build.gradle.kts (1)
30-30: LGTM! Platform upgrade to Ultimate edition is consistent with project-wide changes.The change from
IntellijIdeaCommunitytoIntellijIdeaUltimatealigns with the broader platform upgrade described in the AI summary. This ensures the ASDF plugin will have access to Ultimate edition features.plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java (1)
78-78: Hardcoded YouTrack token committed — revoke immediately and externalize the secretA permanent token is embedded in source. This is a credential leak. Revoke/rotate now and load from runtime (JVM prop/env/CI). Do not commit tokens. Hotfix diff below keeps behavior without exposing secrets.
- private static final String ACCESS_TOKEN = "Bearer perm:YXV0b3JlcG9ydGVy.NjEtMjY=.FgsJOlHXSdCSQTUyfagUKU9WlkrLhV"; + // HOTFIX: never commit tokens; rely on runtime-provided value + private static final String ACCESS_TOKEN = ADMIN_TOKEN;Follow-up (preferred: separate non-admin reporter token). Add near existing token constants:
@VisibleForTesting public static final String REPORT_TOKEN_PROPERTY_KEY = "youtrack.report.token"; public static final String REPORT_TOKEN_VALUE = System.getProperty(REPORT_TOKEN_PROPERTY_KEY); private static final String ACCESS_TOKEN = StringUtil.isEmpty(REPORT_TOKEN_VALUE) ? "" : "Bearer " + REPORT_TOKEN_VALUE;Operational steps:
- Revoke exposed token(s) in YouTrack; rotate and reconfigure CI/IDE run configs.
- Consider history rewrite to purge the secret.
- Add secret scanning in CI to prevent regressions.
#!/bin/bash set -euo pipefail echo "Scanning for leaked YouTrack tokens..." rg -nP --hidden -uu -S '(Bearer\s+perm:|perm:[A-Za-z0-9._-]{16,})' -g '!**/build/**' -g '!**/out/**' -g '!**/target/**' -C2 || trueembedded/src/test/resources/unit/distribution/plugin.none.txt (2)
11-11: LGTM: switched to plain module IDs is consistent.The Module(name=...) → bare identifier change is uniform across dependencies and matches the module naming used elsewhere.
Also applies to: 14-15, 18-19, 22-23
11-11: No remaining Module(name=...) usages — manifests use plain module names.rg found no "Module(name=" occurrences; the generator plugin/testFixtures/src/testFixtures/java/base/PerlDistributionTestCase.kt emits plain "- <module.name>" entries and the plugin.none.txt fixtures (e.g. embedded/src/test/resources/unit/distribution/plugin.none.txt) already use the new format.
mason/mason2/src/test/resources/unit/distribution/plugin.none.txt (2)
11-13: LGTM: formatting aligns with embedded/htmlmason updates.Dependency identifiers are now plain names; indentation and ordering remain stable for tests.
Also applies to: 15-17, 20-21, 24-26
24-26: Sanity-check platform module presence: intellij.platform.frontend.splitgradle.properties sets platformVersion=253 and platformBranch=.20558.43-EAP-SNAPSHOT → target platform = 253.20558.43-EAP-SNAPSHOT. Confirm that intellij.platform.frontend.split is shipped in that IntelliJ build; if not, update the test at mason/mason2/src/test/resources/unit/distribution/plugin.none.txt (lines 24–26) to not assume its presence (make dependency optional or adjust expected output).
mason/htmlmason/src/test/resources/unit/distribution/plugin.none.txt (2)
11-12: LGTM: consistent plain-name deps; ordering preserved.Matches the new convention and keeps deterministic output.
Also applies to: 15-17, 20-22, 25-27
20-22: Confirm upstream uses of these names.Double‑check any parsers or assertions that previously matched
Module(name=...)wrappers are updated here as well; reuse the checks from the embedded file comment.Also applies to: 25-27
mason/mason2/common/build.gradle.kts (1)
44-46: mason/mason2/common/build.gradle.kts — Confirm IU-only target; harden property handling and make edition switchable.
- IntelliJPlatformType.IntellijIdeaUltimate limits the artifact to IU-only; if that’s not intended, make the edition configurable (e.g., -PplatformEdition=IU|IC) or keep Community.
- providers.gradleProperty("useInstaller").get() will throw when the property is absent — use a safe default.
Apply this minimal change:
- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ - useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() + val ideaType = + if (providers.gradleProperty("platformEdition").orNull == "IU") + IntelliJPlatformType.IntellijIdeaUltimate + else + IntelliJPlatformType.IntellijIdeaCommunity + create(ideaType, platformVersionProvider.get()) { + useInstaller = providers.gradleProperty("useInstaller").map(String::toBoolean).getOrElse(false) }Sandbox search failed to confirm repo-wide intent; verify locally with:
rg -nP -g '!**/build/**' 'IntelliJPlatformType\.IntellijIdeaCommunity' rg -nP -g '!**/build/**' 'IntelliJPlatformType\.IntellijIdeaUltimate' rg -nP -g '!**/build/**' 'com\.intellij\.modules\.ultimate'plugin/testFixtures/src/testFixtures/java/base/PerlDistributionTestCase.kt (1)
45-49: Stable rendering of module identifier.Confirm moduleId is a stable string (not a value type whose toString could change). If there’s an explicit string accessor (e.g., id/stringId), prefer it. Also ensure golden files under unit/distribution are updated accordingly.
mason/mason2/frontend/build.gradle.kts (1)
30-30: Switch to Ultimate (IU) acknowledged — repository already targets IU across modules; confirm IU-only intent or make platform configurable.Verification: repo-wide search found numerous create(IntelliJPlatformType.IntellijIdeaUltimate, ...) occurrences (including mason/mason2/frontend/build.gradle.kts); the plugin.xml scan returned no matches for Ultimate-only modules.
If IU-only, keep as-is; if Community (IC) support is required, make the platform type configurable or cross-build to avoid runtime NoClassDefFoundError.mason/framework/common/build.gradle.kts (1)
27-27: Confirm the switch to IntelliJIdeaUltimate (IU) is intentional and update CI/publishing matrices.Sandbox search failed ("No files were searched"), so I couldn't verify. Manually confirm this IU-only change and ensure all modules, run configs, CI jobs, and plugin verification/publishing matrices no longer target IntelliJPlatformType.IntellijIdeaCommunity. Run locally: rg -nP 'IntelliJPlatformType.IntellijIdeaCommunity' -C2 (or git grep) and paste results to re-check.
plugin/cpanminus/common/build.gradle.kts (1)
29-31: Gate switching to IntelliJ IDEA Ultimate and verify CI authFile: plugin/cpanminus/common/build.gradle.kts (lines 29–31)
create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() }
- This change removes Community compatibility and requires authenticated Ultimate downloads in CI; gate the change and confirm CI has the needed JetBrains/Ultimate credentials.
- Make the platform type configurable instead of hardcoding ULT — apply the refactor below.
- val platformVersionProvider: Provider<String> by rootProject.extra - create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + val platformVersionProvider: Provider<String> by rootProject.extra + val platformTypeProvider: Provider<IntelliJPlatformType> = + providers.gradleProperty("platformType") + .map { IntelliJPlatformType.valueOf(it) } + .orElse(IntelliJPlatformType.IntellijIdeaUltimate) + create(platformTypeProvider.get(), platformVersionProvider.get()) { useInstaller = providers.gradleProperty("useInstaller").get().toBoolean() }Verification (the earlier repo search in the sandbox returned no files; run these locally):
# from repo root rg -nP '\bIntelliJPlatformType\.IntellijIdeaCommunity\b' -S || true rg -nP '\bcreate\s*\(\s*IntelliJPlatformType\.' -S || true rg -n '^\s*platformType=' --hidden -S || true rg -nP '\bIntelliJPlatformType\.IntellijIdeaUltimate\b' -S || true rg -n --hidden '(INTELLIJ|JETBRAINS|ULTIMATE|IDEA|DOWNLOAD|TOKEN|CREDENTIAL|LOGIN|AUTH)' .github .circleci || true env | rg -n '(INTELLIJ|JETBRAINS|ULTIMATE|IDEA|DOWNLOAD|TOKEN|CREDENTIAL|LOGIN|AUTH)' || truett2/src/test/resources/unit/distribution/plugin.none.txt (4)
18-19: Frontend deps match the new platform split; confirm these IDs exist in target IDE.
intellij.platform.frontend+perl5.lang.tt2.common.mainaligns with the platform module naming. Please re-run the distribution tests against the intended IDE build to catch missing module IDs early.
22-23: Split frontend chain looks coherent — verified.
tt2/src/test/resources/unit/distribution/plugin.none.txt contains "intellij.platform.frontend.split" (line 22) and "perl5.lang.tt2.frontend.main" (line 23); dependency layering matches expectations.
11-11: Plain module identifiers: LGTM — verify distribution fixtures were migrated consistently.The verification run returned "src/test/resources/unit/distribution: No such file or directory" so the absence of "Module(name=" occurrences couldn't be confirmed. Re-run from the repo root to check:
rg -nP 'Module\s*(name=' -S || true
14-15: Dependency set looks correct; ensure deterministic ordering.File: tt2/src/test/resources/unit/distribution/plugin.none.txt (lines 14–15).
backend.main depends on perl5.plugin.backend.main and perl5.lang.tt2.common.main — if generation doesn't already sort dependencies, sort them to reduce fixture churn.
Automated verification failed in the sandbox (missing tools); manually confirm the dependency lines are consistently ordered.
plugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.kt (1)
26-32: Overall migration looks goodKotlin port is concise; ReadAction usage for bin dirs is appropriate.
mojo/src/test/resources/unit/distribution/plugin.none.txt (3)
22-23: Split-frontend dependency chain: confirm availability and intent.Validate that
intellij.platform.frontend.splitis present in the chosen platform and that depending onperl5.lang.mojo.frontend.main(optional) is intentional for this distribution profile.If either module name changes with future platform upgrades, remember to update this fixture together with the generator to keep tests green.
11-11: Switch to plain module identifiers — verify no remaining Module(name=...) fixturesChange in mojo/src/test/resources/unit/distribution/plugin.none.txt looks good; the verification command returned no output locally — please run this to confirm there are no other legacy entries:
rg -nP 'Module\s*(name=' --glob '/test/resources//distribution/*.txt' -S
18-19: Platform module names: verify against the targeted IntelliJ Platform version (platformVersion=253).gradle.properties sets platformVersion=253 (gradle.properties:20). mojo/src/test/resources/unit/distribution/plugin.none.txt contains
intellij.platform.frontend(lines 18-19); my repo scan returned no matches for that module. Confirmintellij.platform.frontendis present in the IDE distribution's product-info.json / bundle for build 253, or remove the entry from plugin.none.txt to avoid fixture drift on platform bumps.plugin/src/test/resources/unit/distribution/plugin.none.txt (5)
49-51: Ultimate‑gated deps correctly marked under OPTIONALremote-run, profiler, and wsl are ultimate‑gated and remain under OPTIONAL. This matches expected packaging constraints.
Also applies to: 87-90, 96-97
32-33: Resolved — coverage module id verifiedConfirmed in plugin/coverage/src/main/resources/perl5.plugin.coverage.main.xml; no change required.
28-30: Backend/frontend split: confirm IDE build baseline & available modules
- Tokens look correct; ensure generator emits only modules present in the IDE build used by tests.
- Confirm tests run against an IDE build that contains intellij.platform.backend and the frontend split modules; otherwise pin the test baseline.
- Location: plugin/src/test/resources/unit/distribution/plugin.none.txt — lines 28-30 and 54-59.
- Auto-check failed:
rgreturned "unrecognized file type: binary" — unable to locate references; re-run the search or verify manually.
11-12: New perlInstall dependency: confirm packaging intent (no cycle detected)perl5.plugin.perlInstall.main depends only on perl5.plugin.backend.main (plugin/perlInstall/src/main/resources/perl5.plugin.perlInstall.main.xml) — no cycle found. perl5.plugin.asdf.main and perl5.plugin.plenv.main declare perlInstall as a dependency (plugin/asdf/src/main/resources/perl5.plugin.asdf.main.xml, plugin/plenv/src/main/resources/perl5.plugin.plenv.main.xml) and plugin/src/test/resources/unit/distribution/plugin.none.txt shows perlInstall listed under those modules, so enabling asdf/plenv will pull perlInstall into the distribution. Confirm this is intended for the "none" distribution and acceptable to pull perlInstall when asdf/plenv are enabled.
21-21: Resolved — tokens in plugin.none.txt match configured platform (IntelliJIdeaUltimate, platformVersion=253)plugin/src/test/resources/unit/distribution/plugin.none.txt lists Ultimate-related tokens; repository is configured to IntelliJIdeaUltimate (build.gradle.kts creates IntellijIdeaUltimate entries) and gradle.properties sets platformVersion=253.
plugin/build.gradle.kts (1)
30-32: intelliLangModule present — confirm no leftover intelliLangPlugin usages; consider IntellijIdea migration
- gradle.properties:31 — intelliLangModule=intellij.platform.langInjection.
- plugin/intelliLang/build.gradle.kts:35 — bundledModules(providers.gradleProperty("intelliLangModule").get()).
- rg search for "intelliLangPlugin" returned "No files were searched" (inconclusive); run a full repo search or otherwise verify no legacy intelliLangPlugin wiring remains.
- Optional: many files still use IntelliJPlatformType.IntellijIdeaUltimate (e.g., build.gradle.kts:393, plugin/intelliLang/build.gradle.kts:31); migrate to the unified IntellijIdea type to avoid deprecations in 2.9.0.
plugin/perlInstall/build.gradle.kts (1)
28-30: Switch to unified platform type (IntellijIdea) — repo‑wide change; verify plugin metadata
- Replace IntelliJPlatformType.IntellijIdeaUltimate → IntelliJPlatformType.IntellijIdea (occurs in many build.gradle.kts — e.g., plugin/perlInstall/build.gradle.kts:28).
- Before migrating, confirm the intellij-platform-gradle-plugin version supports the unified type and update plugin.xmls as needed: declare required modules (e.g., intellij.platform.langInjection) and ensure since-build/until-build ranges reflect IC vs IU compatibility.
Apply (example):
- create(IntelliJPlatformType.IntellijIdeaUltimate, platformVersionProvider.get()){ + create(IntelliJPlatformType.IntellijIdea, platformVersionProvider.get()){plugin/frontend/split/src/main/java/com/jetbrains/rdclient/actions/base/PerlBackendDelegatingActionCustomization.kt (4)
21-21: LGTM! Necessary import for the new API parameter.The import of
FileTypesupports the new nullable parameter in the overriddenisAvailablemethod.
27-27: LGTM! Clean delegation pattern maintains backward compatibility.The public override correctly implements the new API signature by delegating to the existing private implementation, preserving all existing logic while satisfying the interface contract.
29-29: LGTM! Preserves existing logic with clear visibility.The existing implementation is cleanly extracted to a private method, maintaining the original Registry check and document validation logic without any behavioral changes.
32-33: Confirm ignoring FileType is intentional and safeisAvailable(dataContext, fileType) delegates to the private isAvailable(dataContext) (which checks Registry and PerlFrontendUtil), and getActionCallStrategy(...) also calls isAvailable(dataContext), so the FileType parameter is currently unused. No other implementations of BackendDelegatingActionCustomization were found in the repo — confirm this behavior is deliberate or update to use fileType to avoid missed optimizations.
| override fun getAdditionalProjectLibraries(project: Project): MutableCollection<SyntheticLibrary?> = | ||
| PerlProjectManager.getInstance(project).projectLibraries |
There was a problem hiding this comment.
Return a non-null, immutable collection; filter out nulls
Provider contracts expect non-null SyntheticLibrary entries. Also avoid exposing a mutable internal collection.
Apply:
- override fun getAdditionalProjectLibraries(project: Project): MutableCollection<SyntheticLibrary?> =
- PerlProjectManager.getInstance(project).projectLibraries
+ override fun getAdditionalProjectLibraries(project: Project): Collection<SyntheticLibrary> =
+ PerlProjectManager.getInstance(project).projectLibraries.filterNotNull()📝 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.
| override fun getAdditionalProjectLibraries(project: Project): MutableCollection<SyntheticLibrary?> = | |
| PerlProjectManager.getInstance(project).projectLibraries | |
| override fun getAdditionalProjectLibraries(project: Project): Collection<SyntheticLibrary> = | |
| PerlProjectManager.getInstance(project).projectLibraries.filterNotNull() |
🤖 Prompt for AI Agents
In
plugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.kt
around lines 27 to 28, the method currently returns the mutable projectLibraries
collection which may contain nulls; change it to return a non-null, immutable
collection of SyntheticLibrary by filtering out null entries and returning an
immutable list (e.g. call filterNotNull() and convert to an immutable list) so
callers get a non-null, read-only collection and the internal mutable list is
not exposed.
Qodana for JVM286 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked Detected 13 dependenciesThird-party software listThis page lists the third-party software dependencies used in perl5
Contact Qodana teamContact us at [email protected]
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
plugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.kt (1)
27-28: Return a non-null, immutable collection; filter out nulls.This issue was previously identified: the method returns a mutable collection that may contain nulls, exposing internal state and violating provider contracts that expect non-null entries.
plugin/src/test/resources/unit/distribution/plugin.none.txt (1)
66-68: Remove IntelliLang plugin dependency from module list.We now bundle IntelliLang via modules, so
perl5.plugin.intelliLang.mainmust not depend on theorg.intellij.intelliLangplugin id. Keeping it demands a plugin that’s no longer bundled and will break packaging/runtime. Drop the plugin id and keep the module tokens only.- - org.intellij.intelliLang - perl5.plugin.backend.main - intellij.platform.langInjection
🧹 Nitpick comments (1)
plugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.kt (1)
30-31: Consider splitting the long line for readability.The expression is correct but spans over 130 characters. Breaking it into multiple lines would improve readability.
Example refactor:
- override fun getRootsToWatch(project: Project): List<VirtualFile> = - PerlProjectManager.getInstance(project).allLibraryRoots + ReadAction.compute<List<VirtualFile>, Throwable> { PerlRunUtil.getBinDirectories(project).toList() } + override fun getRootsToWatch(project: Project): List<VirtualFile> { + val libraryRoots = PerlProjectManager.getInstance(project).allLibraryRoots + val binDirectories = ReadAction.compute<List<VirtualFile>, Throwable> { + PerlRunUtil.getBinDirectories(project).toList() + } + return libraryRoots + binDirectories + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (58)
build.gradle.kts(2 hunks)embedded/backend/build.gradle.kts(1 hunks)embedded/build.gradle.kts(1 hunks)embedded/common/build.gradle.kts(1 hunks)embedded/frontend/build.gradle.kts(1 hunks)embedded/frontend/split/build.gradle.kts(1 hunks)embedded/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)gradle.properties(1 hunks)mason/framework/common/build.gradle.kts(1 hunks)mason/framework/frontend/build.gradle.kts(1 hunks)mason/framework/frontend/split/build.gradle.kts(1 hunks)mason/framework/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)mason/htmlmason/common/build.gradle.kts(1 hunks)mason/htmlmason/frontend/build.gradle.kts(1 hunks)mason/htmlmason/frontend/split/build.gradle.kts(1 hunks)mason/htmlmason/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)mason/mason2/common/build.gradle.kts(1 hunks)mason/mason2/frontend/build.gradle.kts(1 hunks)mason/mason2/frontend/split/build.gradle.kts(1 hunks)mason/mason2/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)mojo/common/build.gradle.kts(1 hunks)mojo/frontend/build.gradle.kts(1 hunks)mojo/frontend/split/build.gradle.kts(1 hunks)mojo/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)plugin/asdf/build.gradle.kts(1 hunks)plugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.java(0 hunks)plugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.kt(1 hunks)plugin/berrybrew/build.gradle.kts(1 hunks)plugin/build.gradle.kts(1 hunks)plugin/carton/build.gradle.kts(1 hunks)plugin/common/build.gradle.kts(1 hunks)plugin/common/src/main/java/com/intellij/openapi/projectRoots/impl/PerlModuleExtension.java(1 hunks)plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java(2 hunks)plugin/coverage/build.gradle.kts(1 hunks)plugin/cpan/build.gradle.kts(1 hunks)plugin/cpanminus/backend/build.gradle.kts(1 hunks)plugin/cpanminus/common/build.gradle.kts(1 hunks)plugin/debugger/build.gradle.kts(1 hunks)plugin/frontend/build.gradle.kts(1 hunks)plugin/frontend/split/build.gradle.kts(1 hunks)plugin/frontend/split/src/main/java/com/jetbrains/rdclient/actions/base/PerlBackendDelegatingActionCustomization.kt(1 hunks)plugin/frontend/split/src/main/java/com/jetbrains/rdclient/editorActions/cwm/PerlFrontendEditorActionHandlerStrategyCustomizer.kt(2 hunks)plugin/idea/build.gradle.kts(1 hunks)plugin/intelliLang/build.gradle.kts(1 hunks)plugin/intelliLang/src/main/resources/perl5.plugin.intelliLang.main.xml(1 hunks)plugin/makeMaker/build.gradle.kts(1 hunks)plugin/moduleBuild/build.gradle.kts(1 hunks)plugin/perlInstall/build.gradle.kts(1 hunks)plugin/perlbrew/build.gradle.kts(1 hunks)plugin/plenv/build.gradle.kts(1 hunks)plugin/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)plugin/terminal/build.gradle.kts(1 hunks)plugin/testFixtures/build.gradle.kts(1 hunks)plugin/testFixtures/src/testFixtures/java/base/PerlDistributionTestCase.kt(2 hunks)tt2/common/build.gradle.kts(1 hunks)tt2/frontend/build.gradle.kts(1 hunks)tt2/frontend/split/build.gradle.kts(1 hunks)tt2/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)
💤 Files with no reviewable changes (1)
- plugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.java
🚧 Files skipped from review as they are similar to previous changes (32)
- plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java
- embedded/build.gradle.kts
- mason/framework/frontend/build.gradle.kts
- plugin/makeMaker/build.gradle.kts
- tt2/common/build.gradle.kts
- mason/htmlmason/frontend/split/build.gradle.kts
- plugin/perlInstall/build.gradle.kts
- plugin/testFixtures/src/testFixtures/java/base/PerlDistributionTestCase.kt
- plugin/frontend/split/build.gradle.kts
- plugin/perlbrew/build.gradle.kts
- plugin/berrybrew/build.gradle.kts
- embedded/common/build.gradle.kts
- plugin/build.gradle.kts
- plugin/common/src/main/java/com/intellij/openapi/projectRoots/impl/PerlModuleExtension.java
- tt2/frontend/split/build.gradle.kts
- mojo/frontend/build.gradle.kts
- mason/mason2/src/test/resources/unit/distribution/plugin.none.txt
- plugin/asdf/build.gradle.kts
- embedded/backend/build.gradle.kts
- plugin/moduleBuild/build.gradle.kts
- mason/framework/src/test/resources/unit/distribution/plugin.none.txt
- mason/htmlmason/frontend/build.gradle.kts
- mason/mason2/frontend/split/build.gradle.kts
- plugin/carton/build.gradle.kts
- mason/mason2/common/build.gradle.kts
- mason/framework/common/build.gradle.kts
- mojo/frontend/split/build.gradle.kts
- plugin/intelliLang/src/main/resources/perl5.plugin.intelliLang.main.xml
- embedded/frontend/build.gradle.kts
- embedded/src/test/resources/unit/distribution/plugin.none.txt
- tt2/frontend/build.gradle.kts
- plugin/intelliLang/build.gradle.kts
⏰ 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). (3)
- GitHub Check: Build (windows-latest)
- GitHub Check: Build (macos-latest)
- GitHub Check: Build (ubuntu-latest)
🔇 Additional comments (2)
plugin/frontend/split/src/main/java/com/jetbrains/rdclient/actions/base/PerlBackendDelegatingActionCustomization.kt (1)
21-30: LGTM! Clean delegation pattern for API conformance.The delegation pattern correctly adapts to the new API signature while preserving the original behavior. The public override accepts the
FileType?parameter for API conformance and delegates to the private method that maintains the existing logic. ThefileTypeparameter is intentionally unused, which is appropriate when the implementation doesn't require the additional context.plugin/frontend/split/src/main/java/com/jetbrains/rdclient/editorActions/cwm/PerlFrontendEditorActionHandlerStrategyCustomizer.kt (1)
22-40: LGTM! API signature updated for platform conformance.The method signature correctly extends to accept the optional
FileType?parameter. The parameter is currently unused, which aligns with the API conformance pattern seen across this PR. The existing logic based onactionIdandPerlFrontendUtil.isPluginDocumentremains intact.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
plugin/src/test/resources/unit/distribution/plugin.none.txt (1)
66-68: Remove IntelliLang plugin id to align with module bundling.This expected distribution still lists
org.intellij.intelliLangalongside the newintellij.platform.langInjectionmodule, so the generator will try to provision both. The PR’s switch to module-based bundling means the plugin id must go; otherwise the fixture drifts from the intended packaging and will break the test once generation is corrected. Already noted earlier—please drop the plugin entry here as well.- - org.intellij.intelliLang - perl5.plugin.backend.main - intellij.platform.langInjectionplugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.kt (1)
27-28: Address the past review: Return non-null, immutable collectionThis issue was previously flagged but remains unresolved. The provider API contract expects non-null
SyntheticLibraryentries in a non-null collection. Additionally, exposing the mutable internal collection directly violates encapsulation.Apply this diff:
- override fun getAdditionalProjectLibraries(project: Project): MutableCollection<SyntheticLibrary?> = - PerlProjectManager.getInstance(project).projectLibraries + override fun getAdditionalProjectLibraries(project: Project): Collection<SyntheticLibrary> = + PerlProjectManager.getInstance(project).projectLibraries.filterNotNull()plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java (1)
78-78: Token still hardcoded — previous recommendations not followed.The token was rotated but remains hardcoded in source control. This is the same critical security issue flagged in the previous review. The token must be:
- Revoked immediately in YouTrack (this is the second leak)
- Removed from source and history if feasible
- Externalized using the existing pattern (load from system property like
ADMIN_TOKENon line 77)The previous review provided detailed implementation guidance that should be followed. Simply rotating the token without externalizing it does not resolve the security risk.
Apply the hotfix from the previous review to stop hardcoding tokens:
- private static final String ACCESS_TOKEN = "Bearer perm:YXV0b3JlcG9ydGVy.NjEtMjY=.FgsJOlHXSdCSQTUyfagUKU9WlkrLhV"; + // HOTFIX: load at runtime; do not commit tokens + private static final String ACCESS_TOKEN = ADMIN_TOKEN;Then follow the recommended implementation from the previous review to add a separate report token property with proper guards.
🧹 Nitpick comments (1)
plugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.kt (1)
30-31: Consider narrowing the exception type parameterThe
Throwabletype parameter inReadAction.computeis overly broad. IfPerlRunUtil.getBinDirectoriesonly throws runtime exceptions, consider usingRuntimeExceptionorNothingfor better type safety.Example:
- PerlProjectManager.getInstance(project).allLibraryRoots + ReadAction.compute<List<VirtualFile>, Throwable> { PerlRunUtil.getBinDirectories(project).toList() } + PerlProjectManager.getInstance(project).allLibraryRoots + ReadAction.compute<List<VirtualFile>, RuntimeException> { PerlRunUtil.getBinDirectories(project).toList() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (58)
build.gradle.kts(2 hunks)embedded/backend/build.gradle.kts(1 hunks)embedded/build.gradle.kts(1 hunks)embedded/common/build.gradle.kts(1 hunks)embedded/frontend/build.gradle.kts(1 hunks)embedded/frontend/split/build.gradle.kts(1 hunks)embedded/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)gradle.properties(1 hunks)mason/framework/common/build.gradle.kts(1 hunks)mason/framework/frontend/build.gradle.kts(1 hunks)mason/framework/frontend/split/build.gradle.kts(1 hunks)mason/framework/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)mason/htmlmason/common/build.gradle.kts(1 hunks)mason/htmlmason/frontend/build.gradle.kts(1 hunks)mason/htmlmason/frontend/split/build.gradle.kts(1 hunks)mason/htmlmason/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)mason/mason2/common/build.gradle.kts(1 hunks)mason/mason2/frontend/build.gradle.kts(1 hunks)mason/mason2/frontend/split/build.gradle.kts(1 hunks)mason/mason2/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)mojo/common/build.gradle.kts(1 hunks)mojo/frontend/build.gradle.kts(1 hunks)mojo/frontend/split/build.gradle.kts(1 hunks)mojo/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)plugin/asdf/build.gradle.kts(1 hunks)plugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.java(0 hunks)plugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.kt(1 hunks)plugin/berrybrew/build.gradle.kts(1 hunks)plugin/build.gradle.kts(1 hunks)plugin/carton/build.gradle.kts(1 hunks)plugin/common/build.gradle.kts(1 hunks)plugin/common/src/main/java/com/intellij/openapi/projectRoots/impl/PerlModuleExtension.java(1 hunks)plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java(2 hunks)plugin/coverage/build.gradle.kts(1 hunks)plugin/cpan/build.gradle.kts(1 hunks)plugin/cpanminus/backend/build.gradle.kts(1 hunks)plugin/cpanminus/common/build.gradle.kts(1 hunks)plugin/debugger/build.gradle.kts(1 hunks)plugin/frontend/build.gradle.kts(1 hunks)plugin/frontend/split/build.gradle.kts(1 hunks)plugin/frontend/split/src/main/java/com/jetbrains/rdclient/actions/base/PerlBackendDelegatingActionCustomization.kt(1 hunks)plugin/frontend/split/src/main/java/com/jetbrains/rdclient/editorActions/cwm/PerlFrontendEditorActionHandlerStrategyCustomizer.kt(2 hunks)plugin/idea/build.gradle.kts(1 hunks)plugin/intelliLang/build.gradle.kts(1 hunks)plugin/intelliLang/src/main/resources/perl5.plugin.intelliLang.main.xml(1 hunks)plugin/makeMaker/build.gradle.kts(1 hunks)plugin/moduleBuild/build.gradle.kts(1 hunks)plugin/perlInstall/build.gradle.kts(1 hunks)plugin/perlbrew/build.gradle.kts(1 hunks)plugin/plenv/build.gradle.kts(1 hunks)plugin/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)plugin/terminal/build.gradle.kts(1 hunks)plugin/testFixtures/build.gradle.kts(1 hunks)plugin/testFixtures/src/testFixtures/java/base/PerlDistributionTestCase.kt(2 hunks)tt2/common/build.gradle.kts(1 hunks)tt2/frontend/build.gradle.kts(1 hunks)tt2/frontend/split/build.gradle.kts(1 hunks)tt2/src/test/resources/unit/distribution/plugin.none.txt(1 hunks)
💤 Files with no reviewable changes (1)
- plugin/backend/src/main/java/com/perl5/lang/perl/idea/configuration/settings/sdk/PerlLibraryProvider.java
✅ Files skipped from review due to trivial changes (1)
- mojo/src/test/resources/unit/distribution/plugin.none.txt
🚧 Files skipped from review as they are similar to previous changes (27)
- mason/mason2/frontend/split/build.gradle.kts
- plugin/testFixtures/src/testFixtures/java/base/PerlDistributionTestCase.kt
- plugin/asdf/build.gradle.kts
- plugin/plenv/build.gradle.kts
- tt2/common/build.gradle.kts
- plugin/intelliLang/src/main/resources/perl5.plugin.intelliLang.main.xml
- mason/mason2/src/test/resources/unit/distribution/plugin.none.txt
- mason/framework/src/test/resources/unit/distribution/plugin.none.txt
- embedded/backend/build.gradle.kts
- mason/htmlmason/src/test/resources/unit/distribution/plugin.none.txt
- plugin/carton/build.gradle.kts
- mojo/frontend/build.gradle.kts
- mason/htmlmason/common/build.gradle.kts
- plugin/common/build.gradle.kts
- mason/mason2/frontend/build.gradle.kts
- embedded/frontend/split/build.gradle.kts
- plugin/perlbrew/build.gradle.kts
- plugin/berrybrew/build.gradle.kts
- plugin/cpanminus/backend/build.gradle.kts
- plugin/cpanminus/common/build.gradle.kts
- plugin/frontend/split/src/main/java/com/jetbrains/rdclient/editorActions/cwm/PerlFrontendEditorActionHandlerStrategyCustomizer.kt
- plugin/debugger/build.gradle.kts
- mason/framework/frontend/build.gradle.kts
- mason/framework/common/build.gradle.kts
- plugin/makeMaker/build.gradle.kts
- plugin/common/src/main/java/com/intellij/openapi/projectRoots/impl/PerlModuleExtension.java
- gradle.properties
⏰ 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). (3)
- GitHub Check: Build (windows-latest)
- GitHub Check: Build (ubuntu-latest)
- GitHub Check: Build (macos-latest)
🔇 Additional comments (11)
plugin/frontend/split/src/main/java/com/jetbrains/rdclient/actions/base/PerlBackendDelegatingActionCustomization.kt (3)
21-21: LGTM!The import is necessary to support the new
fileTypeparameter in the updated method signature.
27-28: Verify that ignoring the fileType parameter is intentional.The new
fileTypeparameter is completely unused in the implementation. While this preserves the previous evaluation logic as noted in the summary, please confirm that:
- Ignoring
fileTypedoesn't cause unexpected behavior in the parent class's expectations- The action availability logic doesn't need to consider file type constraints
If file type filtering is relevant for this action, consider incorporating it into the availability check:
- override fun isAvailable(dataContext: DataContext, fileType: FileType?): Boolean = isAvailable(dataContext) + override fun isAvailable(dataContext: DataContext, fileType: FileType?): Boolean = + isAvailable(dataContext) && (fileType == null || /* add file type check if needed */)Otherwise, if ignoring
fileTypeis intentional, consider adding a comment to clarify this design decision for future maintainers.
29-30: LGTM!The private overload correctly preserves the original availability logic, maintaining the registry check and plugin document validation.
mason/htmlmason/frontend/build.gradle.kts (1)
30-31: Confirm intentional Ultimate-only targeting.Switching to
IntellijIdeaUltimatemeans this module (and the resulting plugin) can no longer run on Community builds. Please confirm that restricting compatibility to Ultimate is desired and that any publishing metadata (e.g., plugin.xml since-build/until-build) is updated accordingly.plugin/cpan/build.gradle.kts (1)
30-31: LGTM – matches the Ultimate migration.plugin/terminal/build.gradle.kts (1)
30-31: Looks good – aligns with the Ultimate roll-out.mason/framework/frontend/split/build.gradle.kts (1)
30-31: Change is consistent with the Ultimate migration.embedded/build.gradle.kts (1)
39-41: Ultimate platform selection looks correct here.mason/mason2/common/build.gradle.kts (1)
45-46: LGTM – consistent with the project-wide Ultimate target.mojo/common/build.gradle.kts (1)
45-46: Change aligns with the Ultimate migration plan.plugin/perlInstall/build.gradle.kts (1)
28-29: Looks good – matches the rest of the Ultimate rollout.
| bundledModules( | ||
| providers.gradleProperty("intelliLangModule").get(), | ||
| ) |
There was a problem hiding this comment.
Include the backend module in the production bundle.
Previously bundledPlugins(intelliLang) pulled in every module of the IntelliLang plugin. Now bundledModules(...) lists only intelliLangModule, while test fixtures explicitly add both intelliLangModule and intelliLangBackendModule. Shipping the main plugin without the backend module risks losing IntelliLang functionality at runtime (language injections, inspections, etc.). Please add the backend module here as well to preserve parity with the old packaging.
🤖 Prompt for AI Agents
In plugin/build.gradle.kts around lines 30 to 32, the production bundle
currently only includes intelliLangModule which omits the backend module and
will break runtime IntelliLang features; update the bundledModules call to
include both providers.gradleProperty("intelliLangModule").get() and
providers.gradleProperty("intelliLangBackendModule").get() so the backend is
shipped with the main plugin, matching the test fixture configuration.
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| * Copyright 2015-2025 Alexandr Evstigneev | |||
| * Copyright 2015-2024 Alexandr Evstigneev | |||
There was a problem hiding this comment.
Copyright year should be 2025, not 2024.
The copyright year was changed from 2015-2025 to 2015-2024, but this PR is from September 2025. The copyright should reflect the current year of modification.
Apply this diff to correct the copyright year:
- * Copyright 2015-2024 Alexandr Evstigneev
+ * Copyright 2015-2025 Alexandr Evstigneev📝 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.
| * Copyright 2015-2024 Alexandr Evstigneev | |
| * Copyright 2015-2025 Alexandr Evstigneev |
🤖 Prompt for AI Agents
In plugin/common/src/main/java/com/perl5/errorHandler/YoutrackErrorHandler.java
around line 2, update the copyright year range from "2015-2024" to "2015-2025";
replace the existing year token on line 2 with the corrected "2015-2025" string
so the file reflects the current modification year.
New one was removed and we need to migrate,
1 participant
Summary by CodeRabbit