feat(kotlin-builder): rewrite builtin-plugin -Xplugin to --plugins (with warning)#22
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Elide KotlinBuilder shim to proactively rewrite rules_kotlin-supplied -Xplugin=<jar> flags that duplicate Elide-bundled compiler plugins into Elide’s pinned builtin form (--plugins=<name>), optionally emitting a Bazel-visible warning controlled by a new build setting.
Changes:
- Detect and drop duplicate builtin
-Xpluginjars (currently serialization), replacing them with--plugins=...before the--separator. - Add a warning (suppressible via
//config/kotlinc:warn_builtin_plugin_rewrite=false, wired through--quiet_plugin_rewrite) and plumb the new config through the toolchain launcher + shim config. - Add/extend tests to cover detection, rewrite behavior, warning text, and the new shim flag parsing.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/kotlin_builder/MainTest.kt | Adds coverage for parsing --quiet_plugin_rewrite into shim config. |
| tests/kotlin_builder/ElideCompileTest.kt | Verifies plan() rewrites builtin -Xplugin to --plugins= and preserves non-builtin plugins. |
| tests/kotlin_builder/BuiltinPluginsTest.kt | New unit tests for builtin plugin detection, deduping, ignore behavior, and warning text. |
| tests/kotlin_builder/BUILD.bazel | Registers the new builtin_plugins_test. |
| elide/kotlin/toolchain.bzl | Adds launcher plumbing + attribute for --quiet_plugin_rewrite, driven by a new config_setting select. |
| elide/kotlin/builder/Main.kt | Adds config field + warning emission (unless silenced) into WorkResponse output. |
| elide/kotlin/builder/ElideCompile.kt | Implements the actual rewrite: drop builtin jars and add --plugins=... ahead of --. |
| elide/kotlin/builder/BuiltinPlugins.kt | Introduces builtin plugin mapping/detection and the warning formatter. |
| config/kotlinc/BUILD.bazel | Adds warn_builtin_plugin_rewrite bool_flag and matching config_setting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…arn) When rules_kotlin delivers a `-Xplugin=<jar>` that duplicates a compiler plugin bundled in Elide, Elide warns that the raw jar may version-mismatch the embedded compiler and recommends its builtin. The shim now does that automatically on the fast path: it drops the matching `-Xplugin` and enables the version-pinned builtin via `--plugins=`, and surfaces a single Bazel `[warning]` naming the jar and replacement. Covers all four builtins Elide exposes via `--plugins` (per `elide kotlinc --help`): serialization, power-assert, atomicfu, metro — matched by the compiler-plugin jar basename (prefix-agnostic so kotlin-/kotlinx- both map; runtimes and unrelated plugins are left alone). The warning is on by default and silenceable once understood: `--@rules_elide//config/kotlinc:warn_builtin_plugin_rewrite=false` makes the toolchain launcher pass the shim `--quiet_plugin_rewrite` (the rewrite itself always happens). Tests: BuiltinPlugins detection (all four, both kotlin/kotlinx prefixes, passthrough, dedup), non-builtin/runtime ignore, warning text; ElideCompile.plan drops the serialization -Xplugin and adds --plugins=serialization before `--` while keeping non-builtin jars; Main parses --quiet_plugin_rewrite.
Sam Gammon (sgammon)
force-pushed
the
fix/builtin-plugin-rewrite
branch
from
f9f3816 to
d79749e
Compare
Address Copilot review: builtinFor() and the rewrite warning extracted the
jar basename with substringAfterLast('/') only, so a Windows-style path
(C:\...\foo.jar) in a flagfile would neither match the builtin MAP nor print
a clean basename. Add a separator-tolerant basename() helper used in both,
with a test for backslash paths.
Merging this PR will not alter performance
|
This was referenced Jun 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a
kt_jvm_*target uses a compiler plugin that Elide also bundles (e.g. kotlinx-serialization), rules_kotlin passes-Xplugin=<jar>and Elide emits:Fix (in the shim)
On the fast path, the KotlinBuilder shim now detects a
-Xplugin=<jar>that duplicates an Elide builtin, drops it, and enables the version-pinned builtin via--plugins=<name>instead — exactly what Elide recommends. It surfaces one Bazel-visible[warning]naming the jar and the replacement.--@rules_elide//config/kotlinc:warn_builtin_plugin_rewrite=falsesilences the warning (the toolchain launcher then passes the shim--quiet_plugin_rewrite). The rewrite itself always happens; only the warning is suppressed.-Xpluginis untouched.Tests
BuiltinPluginsTest: detection (classpath + passthrough), dedup, non-builtin ignore, warning text (names jar, replacement, silence flag).ElideCompileTest:plan()drops the serialization-Xpluginand adds--plugins=serializationbefore--; keeps non-builtin jars.MainTest:--quiet_plugin_rewriteparsed intoConfig.//tests/...36/36.