From 0dc503e0afda37281019abecca47013adf991f05 Mon Sep 17 00:00:00 2001
From: Shai Almog <67850168+shai-almog@users.noreply.github.com>
Date: Tue, 1 Sep 2026 11:30:53 +0300
Subject: [PATCH 01/94] Stop an ordinary AndroidX dependency from failing the
build on a duplicate Kotlin class
Kotlin 1.8.0 folded kotlin-stdlib-jdk7 and kotlin-stdlib-jdk8 into
kotlin-stdlib and left the two jdk artifacts as empty shims. Gradle resolves
every module's version on its own, so a graph reaching kotlin-stdlib at 1.8 or
newer through one path and kotlin-stdlib-jdk8 at something older through
another ends up with two real jars carrying the same classes:
Duplicate class kotlin.collections.jdk8.CollectionsJDK8Kt found in modules
kotlin-stdlib-1.8.22.jar (org.jetbrains.kotlin:kotlin-stdlib:1.8.22) and
kotlin-stdlib-jdk8-1.6.21.jar (org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.21)
Nothing Kotlin-shaped in the app is needed to produce it, which is what makes
the report unreadable from the app side: com.android.billingclient:billing:9.1.0
does it on its own, reaching kotlin-stdlib 1.8.22 through
androidx.core:core -> core-ktx and kotlin-stdlib-jdk8 1.6.21 through
androidx.core:core -> lifecycle-runtime -> kotlinx-coroutines-android. The app
declares one library and the failure names two Kotlin artifacts it has never
heard of. Our own androidx.appcompat default puts a second kotlin-stdlib-jdk8
branch in the graph too, so the exposure is not confined to billing.
Gradle would normally sort this out. From 1.9.22 kotlin-stdlib publishes module
metadata constraining kotlin-stdlib-jdk7 and jdk8 to 1.8.0 -- exactly the
alignment added here. The 1.8.x line, which is what the current AndroidX
releases resolve to, publishes no .module file at all, only a POM, and a POM
cannot express a constraint. So on 1.8.x nothing tells Gradle the two artifacts
overlap. This supplies for 1.8.x what JetBrains supplies from 1.9.22 on.
Written as a constraint rather than a force: it raises a version, never lowers
one, and never pulls a module into a graph that does not already contain it. An
app with no Kotlin anywhere resolves exactly as before. Skipped when the Kotlin
Gradle plugin is applied, since it performs the same alignment itself and the
Kotlin version such a build compiles with can be older than the floor. Skipped
when the app already names either jdk artifact or the Kotlin BOM in its own
Gradle build hints, and switchable with android.kotlinStdlibAlignment=false.
Verified against the real graph with Gradle rather than by reading POMs. Both
coordinates resolved from google() + mavenCentral() for billing:9.1.0 plus
appcompat:1.6.1:
without the block: kotlin-stdlib:1.8.22, kotlin-stdlib-jdk7:1.6.21,
kotlin-stdlib-jdk8:1.6.21 <- two real jars, the failure
with the block: kotlin-stdlib:1.8.22, kotlin-stdlib-jdk7:1.8.0,
kotlin-stdlib-jdk8:1.8.0 <- shims, no duplicate
and for an app whose graph has no Kotlin in it, zero org.jetbrains.kotlin
modules either way. jdk7 resolving to 1.6.21 alongside jdk8 is why both are
aligned and not just the one the reports name.
Co-Authored-By: Claude Opus 5 (1M context) The failure it prevents. Kotlin 1.8.0 folded the contents of
+ * {@code kotlin-stdlib-jdk7} and {@code kotlin-stdlib-jdk8} into
+ * {@code kotlin-stdlib} and left the two jdk artifacts as empty shims that
+ * only depend on it. Gradle resolves every module's version independently,
+ * so a graph that asks for {@code kotlin-stdlib} at 1.8.0 or newer through
+ * one path and {@code kotlin-stdlib-jdk8} at something older through
+ * another ends up with two real jars carrying the same classes, and the
+ * build dies in {@code checkReleaseDuplicateClasses}: Nothing exotic is needed to produce it. A single ordinary dependency
+ * does it on its own: {@code com.android.billingclient:billing:9.1.0} pulls
+ * {@code androidx.core:core:1.15.0}, which reaches {@code kotlin-stdlib}
+ * 1.8.22 through {@code core-ktx} and {@code kotlin-stdlib-jdk8} 1.6.21
+ * through {@code lifecycle-runtime -> kotlinx-coroutines-android:1.6.4}.
+ * Neither coordinate is anything Codename One asked for, which is what makes
+ * the error so hard to read from the app side: the app declares one library
+ * and the report names two Kotlin artifacts it has never heard of. Why Gradle does not sort this out by itself. It normally would.
+ * From 1.9.22 {@code kotlin-stdlib} publishes Gradle module metadata whose
+ * {@code jvmApiElements} and {@code jvmRuntimeElements} variants carry
+ * dependency constraints raising {@code kotlin-stdlib-jdk7} and
+ * {@code kotlin-stdlib-jdk8} to {@value #MERGED_STDLIB_FLOOR} -- the exact
+ * alignment below. The 1.8.x line, which is what the current AndroidX
+ * releases resolve to, publishes no {@code .module} file at all, only
+ * a POM, and a POM cannot express a constraint. So on 1.8.x there is nothing
+ * telling Gradle the two artifacts overlap, and it has no way to find out.
+ * This class supplies for 1.8.x what JetBrains supplies from 1.9.22 on. Why a constraint and not a force. A constraint raises a version
+ * and never lowers one, and never pulls a module into a graph that does not
+ * already contain it. An app with no Kotlin anywhere is therefore completely
+ * unaffected -- the block resolves to nothing. An app that does have the jdk
+ * artifacts gets them at {@value #MERGED_STDLIB_FLOOR} or newer, which is
+ * always a shim, so the duplicate cannot arise whichever version of
+ * {@code kotlin-stdlib} the rest of the graph settles on. Forcing a fixed
+ * version would instead override a newer one the app deliberately asked
+ * for. Why it is skipped when the Kotlin Gradle plugin is applied. The
+ * plugin performs this same alignment itself
+ * ({@code kotlin.stdlib.jdk.variants.version.alignment}, on by default), so
+ * emitting ours would be redundant there. More to the point, an app with
+ * {@code .kt} sources is compiled by whatever Kotlin version the build
+ * selected, and that can be older than {@value #MERGED_STDLIB_FLOOR} --
+ * pushing a newer stdlib underneath an older compiler earns a
+ * "runtime version is newer than compiler" warning for no gain. Leaving the
+ * plugin to do its own job avoids both. Extracted into a pure static helper so it is unit-testable without a
+ * Gradle run and so the BuildDaemon copy stays trivially diffable --
+ * keep this file in sync with its twin in the other repository. It is also the exact floor {@code kotlin-stdlib:1.9.22}'s own module
+ * metadata constrains them to, so this is JetBrains' number rather than
+ * one chosen here. Every case here is about restraint rather than about the block's text:
+ * the alignment lands in the dependency graph of every AndroidX app, so the
+ * cases that must produce nothing matter more than the one that must produce
+ * something. The floor gets a test of its own because 1.8.0 is not a
+ * preference -- it is the release where the two artifacts became empty
+ * shims, and lowering it would reintroduce the duplicate class the block
+ * exists to prevent. Source text, because the expression is a local inside a method
+ * thousands of lines long that cannot be called without a whole staged
+ * Android project.
+ * Duplicate class kotlin.collections.jdk8.CollectionsJDK8Kt found in modules
+ * kotlin-stdlib-1.8.22.jar (org.jetbrains.kotlin:kotlin-stdlib:1.8.22) and
+ * kotlin-stdlib-jdk8-1.6.21.jar (org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.21)
+ *
+ *
+ *
Why it is skipped when the Kotlin Gradle plugin is applied. The - * plugin performs this same alignment itself - * ({@code kotlin.stdlib.jdk.variants.version.alignment}, on by default), so - * emitting ours would be redundant there. More to the point, an app with - * {@code .kt} sources is compiled by whatever Kotlin version the build - * selected, and that can be older than {@value #MERGED_STDLIB_FLOOR} -- - * pushing a newer stdlib underneath an older compiler earns a - * "runtime version is newer than compiler" warning for no gain. Leaving the - * plugin to do its own job avoids both.
+ *Why the Kotlin Gradle plugin only sometimes excuses this. From + * 1.8.0 the plugin aligns the jdk variants itself, so the block would be a + * no-op and is skipped. An older plugin does not, and skipping there was a + * real bug: the versions were resolved with Gradle rather than reasoned + * about, and a project on the {@code android.useGradle8=false} path -- where + * this builder selects Kotlin 1.7.22 -- resolves like this:
+ * + *+ * plugin 1.7.22 alone stdlib 1.7.22 + jdk7/jdk8 1.7.22 no duplicate + * plugin 1.7.22 + billing 9.1.0 stdlib 1.8.22 + jdk7/jdk8 1.7.22 DUPLICATE + * the same, with this block stdlib 1.8.22 + jdk7/jdk8 1.8.0 fixed + *+ * + *
Note the middle row is worse than a transitive accident: the 1.7.x + * plugin adds {@code kotlin-stdlib-jdk8} at its own version, so the + * older real jar is guaranteed present rather than merely possible, and any + * dependency that reaches a merged stdlib collides with it. Hence the test + * is the applied plugin's version, not whether a plugin is applied at all, + * and an unreadable version counts as "does not align" so the block is + * written rather than skipped.
+ * + *The cost of that, stated plainly. On the same pre-1.8 plugin + * path, an app whose graph contains no merged stdlib (the first row above) + * did not need the block, and gets its stdlib family raised to + * {@value #MERGED_STDLIB_FLOOR} anyway -- newer than the compiler in use, + * which Kotlin warns about. That is deliberate. Gradle cannot express a + * constraint conditional on what another module resolved to, so the choice + * is between a warning in the case that did not need help and a failed build + * in the case that did, and a warning is the better of the two. Raising the + * builder's own pre-Gradle-8 Kotlin default would remove even that, and is a + * bigger change than this one should carry.
* *Extracted into a pure static helper so it is unit-testable without a * Gradle run and so the BuildDaemon copy stays trivially diffable -- @@ -96,23 +118,39 @@ public class KotlinStdlibAlignment { * {@code kotlin-stdlib-jdk8:1.8.0} carries one and none of them. * *
It is also the exact floor {@code kotlin-stdlib:1.9.22}'s own module - * metadata constrains them to, so this is JetBrains' number rather than - * one chosen here.
+ * metadata constrains them to, and the version from which the Kotlin + * Gradle plugin performs this alignment itself, so this is JetBrains' + * number in three separate places rather than one chosen here. */ public static final String MERGED_STDLIB_FLOOR = "1.8.0"; /** - * Coordinates that mean the app is already managing the Kotlin stdlib - * artifacts itself, in which case this class stays out of the way. - * A BOM counts: it aligns the whole {@code org.jetbrains.kotlin} group, - * which is a superset of what the constraints below do. + * The two artifacts whose classes moved into {@code kotlin-stdlib}. + * + *Both are aligned, and each is suppressed on its own. Suppressing + * both because the app named one would leave the artifact it did not name + * unconstrained, and that is not symmetrical: {@code jdk8} depends on + * {@code jdk7}, so an app pinning jdk8 raises jdk7 with it, while an app + * pinning jdk7 leaves jdk8 exactly where the graph put it -- the original + * duplicate, intact, with the block that would have fixed it switched + * off. They can be treated separately because their class sets are + * disjoint ({@code kotlin.jdk7} / {@code kotlin.io.path} against + * {@code kotlin.collections.jdk8} / {@code kotlin.streams.jdk8}), so + * constraining one and not the other cannot make a new duplicate.
*/ - private static final String[] APP_MANAGED_MARKERS = { + private static final String[] ALIGNED_ARTIFACTS = { "kotlin-stdlib-jdk7", - "kotlin-stdlib-jdk8", - "kotlin-bom" + "kotlin-stdlib-jdk8" }; + /** + * The marker that suppresses the whole block rather than one artifact. + * A BOM aligns every module in the {@code org.jetbrains.kotlin} group, + * which is a superset of what this class does, so an app using one has + * already answered the question for both artifacts. + */ + private static final String KOTLIN_BOM = "kotlin-bom"; + private KotlinStdlibAlignment() { } @@ -125,24 +163,26 @@ private KotlinStdlibAlignment() { * constraints on, {@code implementation} on any AndroidX project. The * caller passes the same name it uses for the rest of the block so a * legacy {@code compile} project stays consistent with itself. - * @param kotlinGradlePluginApplied whether this build applies the Kotlin - * Gradle plugin, which does the alignment itself. Nothing is emitted - * when it does. + * @param appliedKotlinPluginVersion the version of the Kotlin Gradle + * plugin this build applies, or null/empty when it applies none. Only + * {@value #MERGED_STDLIB_FLOOR} and newer align the jdk variants + * themselves; anything older, or anything this cannot read, is treated + * as not aligning and the block is written. * @param appGradleFragments the Gradle text the app itself contributed * ({@code gradleDependencies}, {@code android.gradleDep} and the like). - * An app already naming one of the jdk artifacts or the Kotlin BOM has - * made a deliberate choice and is left alone. Null entries are ignored. + * An artifact the app names there is left to the app; the Kotlin BOM + * suppresses both. Null entries are ignored. * @return the block, newline terminated, or {@code ""} */ public static String constraintsBlock(String configuration, - boolean kotlinGradlePluginApplied, String... appGradleFragments) { - if (kotlinGradlePluginApplied) { + String appliedKotlinPluginVersion, String... appGradleFragments) { + if (alignsItsOwnJdkVariants(appliedKotlinPluginVersion)) { return ""; } if (configuration == null || configuration.trim().length() == 0) { return ""; } - if (appManagesKotlinStdlib(appGradleFragments)) { + if (contains(KOTLIN_BOM, appGradleFragments)) { return ""; } String config = configuration.trim(); @@ -154,49 +194,81 @@ public static String constraintsBlock(String configuration, + "Gradle module metadata to say so, so these are raised to the " + "empty shims to avoid a duplicate class in checkDuplicateClasses"; StringBuilder out = new StringBuilder(); - out.append(" constraints {\n"); for (int i = 0; i < ALIGNED_ARTIFACTS.length; i++) { + if (contains(ALIGNED_ARTIFACTS[i], appGradleFragments)) { + continue; + } out.append(" ").append(config) .append("('org.jetbrains.kotlin:").append(ALIGNED_ARTIFACTS[i]) .append(':').append(MERGED_STDLIB_FLOOR).append("') {\n") .append(" because '").append(because).append("'\n") .append(" }\n"); } - out.append(" }\n"); - return out.toString(); + if (out.length() == 0) { + return ""; + } + return " constraints {\n" + out + " }\n"; } /** - * The two artifacts whose classes moved into {@code kotlin-stdlib}. - * jdk7 is aligned alongside jdk8 even though jdk8 is the one that shows - * up in the reports: jdk8 depends on jdk7, so an app reaching a - * pre-{@value #MERGED_STDLIB_FLOOR} jdk7 through some other path would - * hit the identical duplicate on {@code kotlin.jdk7.AutoCloseableKt}. + * Whether a Kotlin Gradle plugin of this version aligns the jdk stdlib + * variants on its own, making this class's block a no-op. + * + *Answered from the version rather than from "is a plugin applied", + * because the two differ exactly where it matters. Unknown reads as + * false: a version that cannot be parsed -- an app declaring + * {@code kotlin-gradle-plugin:$kotlin_version} produces one -- must not + * silently switch the alignment off.
*/ - private static final String[] ALIGNED_ARTIFACTS = { - "kotlin-stdlib-jdk7", - "kotlin-stdlib-jdk8" - }; + public static boolean alignsItsOwnJdkVariants(String kotlinPluginVersion) { + if (kotlinPluginVersion == null) { + return false; + } + // Shared with the Health Connect floor check rather than parsed again here: + // it already drops a qualifier, which rounds a prerelease up to its release + // and is the forgiving direction for a floor. + String numeric = HealthManifestFragments.numericVersionPrefix( + kotlinPluginVersion.trim()); + if (numeric == null) { + return false; + } + return compareVersions(numeric, MERGED_STDLIB_FLOOR) >= 0; + } - /** - * Whether the app's own Gradle fragments already pin or align the Kotlin - * stdlib jdk artifacts. - */ - public static boolean appManagesKotlinStdlib(String... appGradleFragments) { + /** Whether any fragment names this coordinate. */ + private static boolean contains(String marker, String[] appGradleFragments) { if (appGradleFragments == null) { return false; } for (int i = 0; i < appGradleFragments.length; i++) { String fragment = appGradleFragments[i]; - if (fragment == null) { - continue; - } - for (int j = 0; j < APP_MANAGED_MARKERS.length; j++) { - if (fragment.contains(APP_MANAGED_MARKERS[j])) { - return true; - } + if (fragment != null && fragment.contains(marker)) { + return true; } } return false; } + + /** Numeric dotted version compare; a missing segment counts as zero. */ + private static int compareVersions(String left, String right) { + String[] l = left.split("\\."); + String[] r = right.split("\\."); + int len = Math.max(l.length, r.length); + for (int i = 0; i < len; i++) { + int a = i < l.length ? parse(l[i]) : 0; + int b = i < r.length ? parse(r[i]) : 0; + if (a != b) { + return a < b ? -1 : 1; + } + } + return 0; + } + + private static int parse(String segment) { + try { + return Integer.parseInt(segment); + } catch (NumberFormatException notANumber) { + return 0; + } + } } diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index 530f52726ad..b442a755bd0 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -24,8 +24,6 @@ import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -35,34 +33,32 @@ *Every case here is about restraint rather than about the block's text: * the alignment lands in the dependency graph of every AndroidX app, so the * cases that must produce nothing matter more than the one that must produce - * something. The floor gets a test of its own because 1.8.0 is not a - * preference -- it is the release where the two artifacts became empty - * shims, and lowering it would reintroduce the duplicate class the block - * exists to prevent.
+ * something. The two that must NOT produce nothing -- + * {@link #aPreMergeKotlinPluginStillGetsTheAlignment()} and + * {@link #pinningOneJdkArtifactLeavesTheOtherConstrained()} -- are the ones + * that caught a real over-suppression, so treat a change that makes either + * pass vacuously as a regression. */ public class KotlinStdlibAlignmentTest { - private static String block() { - return KotlinStdlibAlignment.constraintsBlock("implementation", false); - } - - @Test - public void constrainsBothJdkArtifactsToTheShimFloor() { - String out = block(); - assertTrue(out.contains( - "implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.8.0')")); - assertTrue(out.contains( - "implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.0')")); + return KotlinStdlibAlignment.constraintsBlock("implementation", null); } /** - * jdk8 is the artifact that shows up in the duplicate class reports, but - * it depends on jdk7, so leaving jdk7 alone would move the same failure - * onto {@code kotlin.jdk7.AutoCloseableKt} rather than remove it. + * jdk8 is the artifact the duplicate class reports name, but the real + * graph resolves both to the same old version, so aligning jdk8 alone + * would move the failure onto {@code kotlin.jdk7.AutoCloseableKt} rather + * than remove it. */ @Test - public void doesNotAlignJdk8Alone() { - assertTrue(block().contains("kotlin-stdlib-jdk7")); + public void constrainsBothJdkArtifactsToTheShimFloor() { + String out = block(); + check(out.contains( + "implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.8.0')"), + "jdk7 is aligned"); + check(out.contains( + "implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.0')"), + "jdk8 is aligned"); } /** @@ -71,15 +67,16 @@ public void doesNotAlignJdk8Alone() { */ @Test public void theFloorIsTheVersionWhereTheClassesMoved() { - assertEquals("1.8.0", KotlinStdlibAlignment.MERGED_STDLIB_FLOOR); + check("1.8.0".equals(KotlinStdlibAlignment.MERGED_STDLIB_FLOOR), + "the floor is the version where the classes moved"); } /** It is a constraints block, not a dependency declaration or a force. */ @Test public void declaresConstraintsRatherThanDependencies() { String out = block(); - assertTrue(out.contains("constraints {")); - assertFalse(out.contains("force")); + check(out.contains("constraints {"), "it is a constraints block"); + check(!out.contains("force"), "it constrains rather than forces"); int open = 0; int close = 0; for (int i = 0; i < out.length(); i++) { @@ -89,7 +86,7 @@ public void declaresConstraintsRatherThanDependencies() { close++; } } - assertEquals(open, close); + check(open == close, "the block's braces balance"); } /** @@ -101,37 +98,110 @@ public void declaresConstraintsRatherThanDependencies() { @Test public void everyConstraintCarriesAReason() { String out = block(); - assertEquals(2, countOccurrences(out, "because '")); - assertTrue(out.contains("Codename One")); + check(countOccurrences(out, "because '") == 2, + "both constraints say why they are there"); + check(out.contains("Codename One"), "the reason names who wrote it"); + } + + /** + * From 1.8.0 the Kotlin Gradle plugin aligns the jdk variants itself, so + * the block would be a no-op. + */ + @Test + public void skipsOnlyAKotlinPluginThatAlignsItself() { + check("".equals(KotlinStdlibAlignment.constraintsBlock( + "implementation", "1.8.0")), + "the release that starts aligning is skipped"); + check("".equals(KotlinStdlibAlignment.constraintsBlock( + "implementation", "1.9.22")), + "a newer plugin is skipped"); + check(KotlinStdlibAlignment.alignsItsOwnJdkVariants("2.0.0"), + "a major bump still aligns"); + check(KotlinStdlibAlignment.alignsItsOwnJdkVariants("1.9.22-RC2"), + "a qualifier does not hide an aligning version"); + } + + /** + * The case that made this a version test rather than an is-a-plugin-applied + * test. On the {@code android.useGradle8=false} path the builder selects + * Kotlin 1.7.22, which predates the merge and does not align. Worse, the + * 1.7 plugin ADDS {@code kotlin-stdlib-jdk8} at its own version, so the + * pre-merge real jar is guaranteed present; any dependency reaching a + * merged stdlib then collides with it. Measured with Gradle: plugin 1.7.22 + * plus billing 9.1.0 resolves kotlin-stdlib 1.8.22 beside jdk7/jdk8 + * 1.7.22, which is the duplicate. Skipping there shipped the bug. + */ + @Test + public void aPreMergeKotlinPluginStillGetsTheAlignment() { + check(KotlinStdlibAlignment.constraintsBlock("implementation", "1.7.22") + .contains("kotlin-stdlib-jdk8"), + "a pre-merge plugin still gets the alignment"); + check(!KotlinStdlibAlignment.alignsItsOwnJdkVariants("1.7.22"), + "1.7.22 does not align"); + check(!KotlinStdlibAlignment.alignsItsOwnJdkVariants("1.6.21"), + "1.6.21 does not align"); } /** - * The Kotlin Gradle plugin performs the same alignment itself, and the - * Kotlin version a build compiles with can be older than the floor, so - * pushing a newer stdlib underneath it would only earn a warning. + * An app declaring {@code kotlin-gradle-plugin:$kotlin_version} parses to + * nothing. Unknown must read as "does not align" -- guessing the other way + * switches the fix off silently. */ @Test - public void emitsNothingWhenTheKotlinGradlePluginIsApplied() { - assertEquals("", KotlinStdlibAlignment.constraintsBlock("implementation", true)); + public void anUnreadablePluginVersionStillGetsTheAlignment() { + check(!KotlinStdlibAlignment.alignsItsOwnJdkVariants(""), + "no plugin does not align"); + check(!KotlinStdlibAlignment.alignsItsOwnJdkVariants(null), + "a null version does not align"); + check(!KotlinStdlibAlignment.alignsItsOwnJdkVariants("$kotlin_version"), + "a Gradle variable does not read as aligning"); + check(KotlinStdlibAlignment.constraintsBlock( + "implementation", "$kotlin_version").contains("kotlin-stdlib-jdk8"), + "an unreadable version still gets the alignment"); } + /** + * Suppression is per artifact. jdk8 depends on jdk7, so an app pinning + * jdk8 raises jdk7 with it -- but an app pinning jdk7 leaves jdk8 exactly + * where the graph put it, and dropping the whole block there would leave + * the original duplicate intact with its fix switched off. + */ @Test - public void emitsNothingWhenTheAppPinsAJdkArtifactItself() { - assertEquals("", KotlinStdlibAlignment.constraintsBlock("implementation", false, - " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n")); - assertEquals("", KotlinStdlibAlignment.constraintsBlock("implementation", false, - " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.9.22'\n")); + public void pinningOneJdkArtifactLeavesTheOtherConstrained() { + String pinnedJdk7 = KotlinStdlibAlignment.constraintsBlock( + "implementation", null, + " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.9.22'\n"); + check(pinnedJdk7.contains("kotlin-stdlib-jdk8"), + "pinning jdk7 leaves jdk8 constrained"); + check(!pinnedJdk7.contains("kotlin-stdlib-jdk7:1.8.0"), + "the artifact the app pinned is left to the app"); + + String pinnedJdk8 = KotlinStdlibAlignment.constraintsBlock( + "implementation", null, + " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); + check(pinnedJdk8.contains("kotlin-stdlib-jdk7"), + "pinning jdk8 leaves jdk7 constrained"); + check(!pinnedJdk8.contains("kotlin-stdlib-jdk8:1.8.0"), + "the artifact the app pinned is left to the app"); + + String pinnedBoth = KotlinStdlibAlignment.constraintsBlock( + "implementation", null, + " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.9.22'\n" + + " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); + check("".equals(pinnedBoth), + "an app managing both gets no block at all, not an empty one"); } /** * A BOM aligns the whole {@code org.jetbrains.kotlin} group, which is a - * superset of this block, so an app using one has already answered the - * question. + * superset of this block, so it is the one marker that suppresses both. */ @Test public void emitsNothingWhenTheAppUsesTheKotlinBom() { - assertEquals("", KotlinStdlibAlignment.constraintsBlock("implementation", false, - " implementation platform('org.jetbrains.kotlin:kotlin-bom:1.9.22')\n")); + check("".equals(KotlinStdlibAlignment.constraintsBlock( + "implementation", null, + " implementation platform('org.jetbrains.kotlin:kotlin-bom:1.9.22')\n")), + "an app using the Kotlin BOM is left alone"); } /** @@ -141,23 +211,24 @@ public void emitsNothingWhenTheAppUsesTheKotlinBom() { */ @Test public void ignoresEmptyAndNullFragments() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", false, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, "", null, " implementation 'androidx.appcompat:appcompat:1.6.1'\n"); - assertTrue(out.contains("kotlin-stdlib-jdk8")); - assertFalse(KotlinStdlibAlignment.appManagesKotlinStdlib((String[]) null)); + check(out.contains("kotlin-stdlib-jdk8"), + "an empty or absent hint is not a pin"); } /** - * An unrelated Kotlin coordinate is not a pin. Only the two jdk - * artifacts and the BOM decide who owns the alignment; matching - * "kotlin" loosely would silently switch the fix off for any app that - * happens to use a Kotlin library. + * An unrelated Kotlin coordinate is not a pin. Only the two jdk artifacts + * and the BOM decide who owns the alignment; matching "kotlin" loosely + * would silently switch the fix off for any app that happens to use a + * Kotlin library. */ @Test public void anUnrelatedKotlinDependencyIsNotAPin() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", false, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, " implementation 'org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4'\n"); - assertTrue(out.contains("kotlin-stdlib-jdk8")); + check(out.contains("kotlin-stdlib-jdk8"), + "a coroutines dependency does not switch the alignment off"); } /** @@ -167,15 +238,19 @@ public void anUnrelatedKotlinDependencyIsNotAPin() { */ @Test public void usesTheConfigurationItWasGiven() { - String out = KotlinStdlibAlignment.constraintsBlock("compile", false); - assertTrue(out.contains("compile('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.0')")); - assertFalse(out.contains("implementation(")); + String out = KotlinStdlibAlignment.constraintsBlock("compile", null); + check(out.contains("compile('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.0')"), + "the caller's configuration is used"); + check(!out.contains("implementation("), + "no other configuration is assumed"); } @Test public void emitsNothingWithoutAConfiguration() { - assertEquals("", KotlinStdlibAlignment.constraintsBlock(null, false)); - assertEquals("", KotlinStdlibAlignment.constraintsBlock(" ", false)); + check("".equals(KotlinStdlibAlignment.constraintsBlock(null, null)), + "a null configuration writes nothing"); + check("".equals(KotlinStdlibAlignment.constraintsBlock(" ", null)), + "a blank configuration writes nothing"); } /** @@ -186,8 +261,18 @@ public void emitsNothingWithoutAConfiguration() { @Test public void isNewlineTerminatedForConcatenation() { String out = block(); - assertTrue(out.endsWith("}\n")); - assertTrue(out.startsWith(" constraints {\n")); + check(out.endsWith("}\n"), "it ends its own line"); + check(out.startsWith(" constraints {\n"), "it starts its own line"); + } + + private static int countOccurrences(String haystack, String needle) { + int count = 0; + int at = haystack.indexOf(needle); + while (at >= 0) { + count++; + at = haystack.indexOf(needle, at + needle.length()); + } + return count; } /** @@ -212,13 +297,7 @@ public void theBuilderStillWritesItIntoTheDependenciesBlock() throws Exception { assertTrue(block.contains("+ kotlinStdlibConstraints")); } - private static int countOccurrences(String haystack, String needle) { - int count = 0; - int at = haystack.indexOf(needle); - while (at >= 0) { - count++; - at = haystack.indexOf(needle, at + needle.length()); - } - return count; + private static void check(boolean condition, String message) { + assertTrue(condition, message); } } From 23ff2338d529c1f0c18cf0a5ec2ab2e8c3984d1b Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 13:15:38 +0300 Subject: [PATCH 04/94] Read the Kotlin BOM by version too, not by presence A BOM manages the whole org.jetbrains.kotlin group, jdk7 and jdk8 included, so it looked like the one marker that could safely suppress both constraints. It is not, unless it is new enough: a platform contributes constraints, and the highest version still wins, so a BOM can raise the jdk artifacts but cannot pull kotlin-stdlib back down. Measured against a graph that wants stdlib 1.8.22: no BOM stdlib 1.8.22 + jdk7/jdk8 1.6.21 duplicate kotlin-bom 1.7.22 stdlib 1.8.22 + jdk7/jdk8 1.7.22 STILL a duplicate kotlin-bom 1.9.22 all 1.9.22, jdk artifacts shims safe 1.7.22 + this block stdlib 1.8.22 + jdk7/jdk8 1.8.0 fixed So the BOM is now tested the same way the Kotlin Gradle plugin already is, by version rather than by presence, and through the same predicate -- the two questions turned out to be one question asked about two things. An unreadable version reads as "does not align" for both, so kotlin-bom:$kotlinVersion gets the block rather than silently losing it. Worth recording that the first probe of this said the opposite. A bare Gradle configuration with no attributes cannot select a platform's constraint variant, so the BOM appeared to do nothing at any version and the finding looked wrong. It was the probe that was wrong. Anything measuring platform behaviour needs a configuration with real usage attributes, which is what the numbers above use. Three new cases, all three failing against the previous behaviour. Co-Authored-By: Claude Opus 5 (1M context)Only a new enough one. A BOM raises the jdk artifacts but + * cannot pull {@code kotlin-stdlib} back down, because a platform + * contributes constraints and the highest version still wins. So a + * pre-merge BOM leaves exactly the arrangement this class exists to + * prevent -- measured, with the same graph as the class comment's + * table:
+ * + *+ * no BOM stdlib 1.8.22 + jdk7/jdk8 1.6.21 duplicate + * kotlin-bom 1.7.22 stdlib 1.8.22 + jdk7/jdk8 1.7.22 STILL a duplicate + * kotlin-bom 1.9.22 all 1.9.22, jdk artifacts shims safe + *+ * + *
The BOM is therefore tested by version, exactly like the Kotlin + * Gradle plugin above it, and for the same reason: presence is not + * alignment.
*/ private static final String KOTLIN_BOM = "kotlin-bom"; + /** The coordinate a BOM's version is read from. */ + private static final String KOTLIN_BOM_COORDINATE = + "org.jetbrains.kotlin:kotlin-bom:"; + private KotlinStdlibAlignment() { } @@ -182,7 +203,9 @@ public static String constraintsBlock(String configuration, if (configuration == null || configuration.trim().length() == 0) { return ""; } - if (contains(KOTLIN_BOM, appGradleFragments)) { + if (contains(KOTLIN_BOM, appGradleFragments) + && atOrPastTheMerge( + declaredVersion(KOTLIN_BOM_COORDINATE, appGradleFragments))) { return ""; } String config = configuration.trim(); @@ -221,20 +244,67 @@ public static String constraintsBlock(String configuration, * silently switch the alignment off. */ public static boolean alignsItsOwnJdkVariants(String kotlinPluginVersion) { - if (kotlinPluginVersion == null) { + return atOrPastTheMerge(kotlinPluginVersion); + } + + /** + * Whether a Kotlin version is at or past the release that merged the jdk + * artifacts away, and therefore aligns them wherever it is in force -- + * as the Gradle plugin's version or as a BOM's. + * + *Unknown reads as false everywhere it is used. A version that cannot + * be parsed -- {@code kotlin-gradle-plugin:$kotlin_version} and + * {@code kotlin-bom:$kotlinVersion} both produce one -- must not silently + * switch the alignment off.
+ */ + private static boolean atOrPastTheMerge(String kotlinVersion) { + if (kotlinVersion == null) { return false; } // Shared with the Health Connect floor check rather than parsed again here: // it already drops a qualifier, which rounds a prerelease up to its release // and is the forgiving direction for a floor. String numeric = HealthManifestFragments.numericVersionPrefix( - kotlinPluginVersion.trim()); + kotlinVersion.trim()); if (numeric == null) { return false; } return compareVersions(numeric, MERGED_STDLIB_FLOOR) >= 0; } + /** + * The version an app's own Gradle text declares immediately after + * {@code coordinate}, or null when it declares none there or writes one + * this cannot read -- a Gradle variable rather than a literal. + */ + private static String declaredVersion(String coordinate, + String[] appGradleFragments) { + if (appGradleFragments == null) { + return null; + } + for (int i = 0; i < appGradleFragments.length; i++) { + String fragment = appGradleFragments[i]; + if (fragment == null) { + continue; + } + int at = fragment.indexOf(coordinate); + if (at < 0) { + continue; + } + int from = at + coordinate.length(); + int to = from; + while (to < fragment.length() + && "0123456789.".indexOf(fragment.charAt(to)) >= 0) { + to++; + } + while (to > from && fragment.charAt(to - 1) == '.') { + to--; + } + return to > from ? fragment.substring(from, to) : null; + } + return null; + } + /** Whether any fragment names this coordinate. */ private static boolean contains(String marker, String[] appGradleFragments) { if (appGradleFragments == null) { diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index b442a755bd0..d49f031d700 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -193,15 +193,56 @@ public void pinningOneJdkArtifactLeavesTheOtherConstrained() { } /** - * A BOM aligns the whole {@code org.jetbrains.kotlin} group, which is a - * superset of this block, so it is the one marker that suppresses both. + * A BOM manages the whole {@code org.jetbrains.kotlin} group, jdk7 and + * jdk8 included, so a new enough one is the single marker that suppresses + * both constraints. + * + *New enough is the whole point. A BOM raises the jdk artifacts but + * cannot pull {@code kotlin-stdlib} down -- a platform contributes + * constraints and the highest version still wins -- so a pre-merge BOM + * leaves a merged stdlib beside class-bearing jdk jars, which is the + * duplicate. Measured: kotlin-bom 1.7.22 against a graph wanting stdlib + * 1.8.22 resolves jdk7/jdk8 to 1.7.22, still class-bearing.
*/ @Test public void emitsNothingWhenTheAppUsesTheKotlinBom() { check("".equals(KotlinStdlibAlignment.constraintsBlock( "implementation", null, " implementation platform('org.jetbrains.kotlin:kotlin-bom:1.9.22')\n")), - "an app using the Kotlin BOM is left alone"); + "an app using a merged-era Kotlin BOM is left alone"); + check("".equals(KotlinStdlibAlignment.constraintsBlock( + "implementation", null, + " implementation platform('org.jetbrains.kotlin:kotlin-bom:1.8.0')\n")), + "the BOM at the merge itself is enough"); + } + + /** + * The counterpart, and the reason the BOM is read by version rather than + * by presence: a pre-merge BOM does not make the graph safe, so it must + * not switch the block off. + */ + @Test + public void aPreMergeKotlinBomStillGetsTheAlignment() { + String out = KotlinStdlibAlignment.constraintsBlock( + "implementation", null, + " implementation platform('org.jetbrains.kotlin:kotlin-bom:1.7.22')\n"); + check(out.contains("kotlin-stdlib-jdk7:1.8.0"), + "a pre-merge BOM still gets jdk7 aligned"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), + "a pre-merge BOM still gets jdk8 aligned"); + } + + /** + * A BOM whose version is a Gradle variable reads as unknown, and unknown + * must not suppress -- the same fail-safe the plugin version gets. + */ + @Test + public void anUnreadableBomVersionStillGetsTheAlignment() { + String out = KotlinStdlibAlignment.constraintsBlock( + "implementation", null, + " implementation platform(\"org.jetbrains.kotlin:kotlin-bom:$kotlinVersion\")\n"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), + "an unreadable BOM version still gets the alignment"); } /** From e1f4b141c095ed2e0ff5351c48a7f2d67c27f8db Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 13:25:05 +0300 Subject: [PATCH 05/94] Suppress on an active declaration, not on the artifact name appearing anywhere Two review findings, one on each repo, with the same root cause: the check asked whether a string occurs in the app's Gradle text rather than whether the app actually declares anything. Both near misses switch the alignment off for an app that pinned nothing, which is the duplicate-class failure this change exists to prevent: // implementation platform('org.jetbrains.kotlin:kotlin-bom:1.9.22') exclude group: 'org.jetbrains.kotlin', module: 'kotlin-stdlib-jdk8' The first is not a declaration. The second is the opposite of one -- a Gradle exclusion applies only to the dependency edge it is written on, so an independent path still brings the class-bearing jar it names. Comments are now stripped before matching and exclusion lines are dropped, and a declaration has to be spelled as one: the colon-joined coordinate or the map form. Anything else falls through to "not declared", which is the safe direction -- emitting a constraint the app did not need only raises an artifact to a shim, while skipping one it did need fails the build. The BOM's version is read from the same active text, so a commented-out BOM cannot supply the version that suppresses the block. One trap worth naming, because the obvious implementation has it: stripping from every "//" cuts `maven { url 'https://...' }` in half, and these fragments do carry repository URLs. A "//" only opens a comment when it does not follow a colon, and there is a case pinning that. Four new cases. Two fail against the previous behaviour; the other two pin things the stricter matching could plausibly have broken -- the map form still counting as a pin, and a URL still not being a comment. Co-Authored-By: Claude Opus 5 (1M context)The difference is the whole point, because both near misses produce + * the failure this class exists to prevent -- suppressing the constraint + * for an app that never pinned anything:
+ * + *
+ * // implementation platform('org.jetbrains.kotlin:kotlin-bom:1.9.22')
+ * exclude group: 'org.jetbrains.kotlin', module: 'kotlin-stdlib-jdk8'
+ *
+ *
+ * The first is not a declaration at all. The second is the opposite of + * one: a Gradle exclusion applies only to the dependency edge it is + * written on, so an independent path can still bring the class-bearing jar + * it names. Neither may switch the alignment off.
+ * + *Two spellings count as a declaration -- the colon-joined coordinate + * and the map form -- because those are what a pin is actually written as. + * Anything else falls through to "not declared", which is the safe + * direction: emitting a constraint the app did not need only raises an + * artifact to a shim, while skipping one it did need fails the build.
+ */ + private static boolean declaresArtifact(String artifact, String[] appGradleFragments) { if (appGradleFragments == null) { return false; } for (int i = 0; i < appGradleFragments.length; i++) { - String fragment = appGradleFragments[i]; - if (fragment != null && fragment.contains(marker)) { - return true; + String[] lines = activeLines(appGradleFragments[i]); + for (int j = 0; j < lines.length; j++) { + if (declaresArtifactOnLine(artifact, lines[j])) { + return true; + } } } return false; } + private static boolean declaresArtifactOnLine(String artifact, String line) { + if (line.contains(KOTLIN_GROUP + ":" + artifact)) { + return true; + } + // group: 'org.jetbrains.kotlin', name: 'kotlin-stdlib-jdk8', version: '...' + return line.contains(KOTLIN_GROUP) + && (line.contains("name: '" + artifact + "'") + || line.contains("name: \"" + artifact + "\"")); + } + + /** + * A fragment's lines with comments removed and exclusions dropped -- the + * text that actually declares something. + * + *A line comment is only a comment when the {@code //} does not follow + * a colon: {@code maven { url 'https://...' }} is an ordinary declaration + * that a naive strip would cut in half, and these fragments really do + * carry repository URLs.
+ */ + private static String[] activeLines(String fragment) { + if (fragment == null) { + return new String[0]; + } + StringBuilder out = new StringBuilder(); + boolean inBlockComment = false; + for (int i = 0; i < fragment.length(); i++) { + char c = fragment.charAt(i); + if (inBlockComment) { + if (c == '*' && i + 1 < fragment.length() && fragment.charAt(i + 1) == '/') { + inBlockComment = false; + i++; + } else if (c == '\n') { + out.append(c); + } + continue; + } + if (c == '/' && i + 1 < fragment.length()) { + char next = fragment.charAt(i + 1); + if (next == '*') { + inBlockComment = true; + i++; + continue; + } + if (next == '/' && (i == 0 || fragment.charAt(i - 1) != ':')) { + while (i < fragment.length() && fragment.charAt(i) != '\n') { + i++; + } + out.append('\n'); + continue; + } + } + out.append(c); + } + String[] lines = out.toString().split("\n"); + ListThe list is checked against the builder's source rather than + * re-derived, because the failure is an omission and an omission is + * invisible to a test that only exercises what is passed.
+ */ + @Test + public void theBuilderPassesEveryAppControlledDependencyFragment() throws Exception { + byte[] bytes = java.nio.file.Files.readAllBytes(new java.io.File( + "src/main/java/com/codename1/builders/AndroidGradleBuilder.java").toPath()); + String src = new String(bytes, "UTF-8"); + int at = src.indexOf("KotlinStdlibAlignment.constraintsBlock("); + check(at >= 0, "the builder calls the alignment"); + // To the statement terminator, not to "));" -- that lands on the closing paren + // of the LAST argument and slices it in half, so the final fragment never + // matched and the check failed for the wrong reason. + String call = src.substring(at, src.indexOf(";", at)); + // The call form, not the bare hint name: the comment above the argument list + // names android.supportv4Dep too, so matching the name alone passed with the + // argument deleted. Checked by deleting it, which is the only way that kind of + // vacuity shows up. + String[] fragments = { + "additionalDependencies,", + "aiExtraGradleDependencies.toString(),", + "request.getArg(\"android.gradleDep\", \"\")", + "request.getArg(\"android.supportv4Dep\", \"\")", + "request.getArg(\"android.xgradle\", \"\")", + }; + for (String fragment : fragments) { + check(call.contains(fragment), + "the alignment is not told about " + fragment + + ", which reaches the generated dependencies block"); + } + } + /** * A commented-out declaration is not a declaration. The same hazard the * VPN manifest checks cover, in the same builder's hint text: a developer From 5a67ee4257d5b190b7de7b9646161954938d7f48 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 14:04:25 +0300 Subject: [PATCH 07/94] Read declarations on the configuration being constrained, and past commented plugins Two more findings of the same shape as the last round -- text that mentions a coordinate is not a declaration that governs the build -- plus one gate left where it is, on purpose. A declaration on a variant or test configuration does not reach the one the constraints are written on. debugImplementation of a new-enough BOM constrains the debug variant alone, so suppressing on it removed the constraint from the release build that still needed it, and the release build is the one that ships. The variant forms camel-case the configuration they derive from, so requiring the configuration's own lowercase spelling excludes debugImplementation, releaseImplementation and testImplementation without listing them and cannot be defeated by a variant name nobody thought of. api is accepted alongside it, since an api declaration is a real pin on the main variant. The Kotlin plugin version had the same hole through a different parser. HealthManifestFragments takes the first bare substring match, so a commented-out 1.8+ plugin sitting above an active 1.7.x one was read as the applied version and the alignment was skipped for a build whose real plugin does not align. The builder strips comments before parsing now, through the same helper this class already used on its own fragments. Not taken: widening the gate from Gradle 6 to 4.6. The reasoning is now a comment on the gate rather than only in a review thread, because the next reader will ask the same question. 4.6 selects AGP 3.2.0, which cannot compile against a compileSdk the current AndroidX releases require, and that path is given appcompat 1.0.0, whose graph carries no Kotlin at all -- a merged stdlib cannot appear there. Widening would put an untested constraints block into AGP 3.x builds that work today to fix a clash they cannot have, and the two directions are not symmetrical: too narrow leaves an ancient build with a failure it already had, too wide breaks a build that currently succeeds. Three new cases, and the two behaviours were checked by reverting each in turn. Co-Authored-By: Claude Opus 5 (1M context)A declaration on a variant or test configuration does not. + * {@code debugImplementation platform('...kotlin-bom:1.9.22')} constrains + * the debug variant alone, so treating it as the app managing the stdlib + * removes the constraint from the release build that still needs it -- + * and the release build is the one that ships.
+ * + *The variant forms camel-case the configuration they derive from, so + * requiring the configuration's own lowercase spelling excludes + * {@code debugImplementation}, {@code releaseImplementation} and + * {@code testImplementation} without listing them, and cannot be defeated + * by a variant name nobody thought of. {@code api} is accepted alongside + * it because an api declaration is a real pin on the main variant; its + * variant forms are camel-cased in the same way.
+ */ + private static boolean declaresOnTheConstrainedConfiguration(String configuration, + String line) { + if (line.contains(configuration)) { + return true; + } + int at = line.indexOf("api"); + while (at >= 0) { + boolean startsToken = at == 0 || !Character.isLetterOrDigit(line.charAt(at - 1)); + int after = at + "api".length(); + boolean endsToken = after < line.length() + && (line.charAt(after) == ' ' || line.charAt(after) == '('); + if (startsToken && endsToken) { + return true; + } + at = line.indexOf("api", at + 1); + } + return false; + } + + /** + * A Gradle fragment with its comments removed, for a caller that has to + * read a version out of it. + * + *Exposed because the builder parses {@code android.topDependency} for + * the Kotlin plugin version with a helper that takes the first bare + * substring match, so a commented-out declaration above an active one wins + * and decides the alignment. Same hazard as the one this class already + * guards against on its own fragments, reached through a different + * parser.
+ */ + public static String activeText(String fragment) { + String[] lines = activeLines(fragment); + StringBuilder out = new StringBuilder(); + for (int i = 0; i < lines.length; i++) { + out.append(lines[i]).append('\n'); + } + return out.toString(); + } + /** * A fragment's lines with comments removed and exclusions dropped -- the * text that actually declares something. diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index ee518c0b1e3..150decde7cb 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -306,6 +306,69 @@ public void aCommentedOutDeclarationIsNotADeclaration() { "a block-commented pin does not suppress"); } + /** + * activeText strips comments, and the builder has to use it on + * android.topDependency before the plugin version is parsed out of it. + * HealthManifestFragments takes the FIRST bare substring match, so a + * commented-out 1.8+ plugin above an active 1.7.x one is read as the + * applied version and the alignment is skipped for a build that needs it. + */ + @Test + public void aCommentedOutPluginIsNotTheAppliedPlugin() throws Exception { + String topDependency = + "// classpath 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.9.22'\n" + + "classpath 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.7.22'\n"; + check(KotlinStdlibAlignment.activeText(topDependency) + .indexOf("1.9.22") < 0, + "the commented plugin is gone from the active text"); + check(KotlinStdlibAlignment.activeText(topDependency) + .indexOf("1.7.22") >= 0, + "the active plugin survives"); + + byte[] bytes = java.nio.file.Files.readAllBytes(new java.io.File( + "src/main/java/com/codename1/builders/AndroidGradleBuilder.java").toPath()); + String builderSrc = new String(bytes, "UTF-8"); + check(builderSrc.contains("KotlinStdlibAlignment.activeText("), + "the builder strips comments before reading the plugin version"); + } + + /** + * A declaration on a variant or test configuration does not reach the one + * the constraints are written on, so it cannot stand in for a pin. + * debugImplementation of a new-enough BOM constrains the debug variant + * alone -- suppressing on it removes the constraint from the release build + * that still needs it, and the release build is the one that ships. + */ + @Test + public void aVariantOnlyDeclarationDoesNotSuppress() { + String debugBom = KotlinStdlibAlignment.constraintsBlock("implementation", null, + " debugImplementation platform('org.jetbrains.kotlin:kotlin-bom:1.9.22')\n"); + check(debugBom.contains("kotlin-stdlib-jdk8:1.8.0"), + "a debug-only BOM does not suppress the main variant"); + + String testPin = KotlinStdlibAlignment.constraintsBlock("implementation", null, + " testImplementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); + check(testPin.contains("kotlin-stdlib-jdk8:1.8.0"), + "a test-only pin does not suppress the main variant"); + + String releasePin = KotlinStdlibAlignment.constraintsBlock("implementation", null, + " releaseImplementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); + check(releasePin.contains("kotlin-stdlib-jdk8:1.8.0"), + "even a release-only pin is not the configuration being constrained"); + } + + /** + * api is a real pin on the main variant and is honoured, so the + * configuration filter did not narrow to a single keyword. + */ + @Test + public void anApiDeclarationCountsAsAPin() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + " api 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); + check(!out.contains("kotlin-stdlib-jdk8:1.8.0"), "api pins jdk8"); + check(out.contains("kotlin-stdlib-jdk7:1.8.0"), "and leaves jdk7 constrained"); + } + /** * An exclusion is the opposite of a pin. It applies only to the dependency * edge it is written on, so an independent path still brings the From ae92d41196c55ef36d95e4cbe220371b1545fa2c Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 14:13:15 +0300 Subject: [PATCH 08/94] Group physical lines into statements before deciding what the app declared Two findings, both the same crude normalization rather than two bugs, and both ending the same way: an app's explicit pin ignored and the constraint written over the top of it, which is the opposite of what naming the artifact in a build hint is documented to do. implementation( configuration and coordinate 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22' on different lines ) implementation('...kotlin-stdlib-jdk8:1.7.22') { exclude group: 'x' } a real pin, dropped whole for containing "exclude" So a line whose parentheses are still open is joined to the next, and a statement is truncated at "exclude" rather than discarded: what precedes an exclusion is the declaration, what follows it is the part that must not count. A bare exclude line truncates to nothing, so it is still not a pin -- that case kept its test and still passes. Parenthesis counting ignores anything inside a string literal, so a coordinate carrying one cannot unbalance it. Text left with parentheses open at the end of a fragment is unbalanced Gradle, and its lines are kept separate rather than glued into one: gluing would let a configuration from one statement and a coordinate from another read as a single declaration, and suppression is the direction that must never be reached by accident. That has its own case. Four new cases. The two the review named fail against the previous behaviour; the other two pin the ways this fix could have gone wrong. Co-Authored-By: Claude Opus 5 (1M context)Regrouping into statements is {@link #statements}; this method only + * removes the comments.
*/ private static String[] activeLines(String fragment) { if (fragment == null) { @@ -474,17 +477,97 @@ private static String[] activeLines(String fragment) { } out.append(c); } - String[] lines = out.toString().split("\n"); - ListTwo things a per-physical-line check gets wrong, both of which end + * with an app's explicit pin ignored and the constraint written over the + * top of it -- the opposite of what naming the artifact in a build hint + * is documented to do:
+ * + *
+ * implementation( configuration and coordinate
+ * 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22' land on different lines
+ * )
+ *
+ * implementation('...kotlin-stdlib-jdk8:1.7.22') { exclude group: 'x' }
+ * a real pin, dropped whole
+ * for containing "exclude"
+ *
+ *
+ * So a line whose parentheses are still open is joined to the next, and + * a statement is truncated at {@code exclude} rather than discarded -- what + * precedes the exclusion is the declaration, and what follows it is the + * part that must not count. A bare {@code exclude} line truncates to + * nothing and so is still not a pin.
+ * + *Joining stops at the end of the fragment: text left with parentheses + * open is unbalanced Gradle, and rather than glue the remainder into one + * long line -- which would make unrelated statements look like a single + * declaration, and suppression is the direction that must never be reached + * by accident -- its lines are kept as they were.
+ */ + private static String[] statements(String[] lines) { + ListThe variant forms camel-case the configuration they derive from, so - * requiring the configuration's own lowercase spelling excludes - * {@code debugImplementation}, {@code releaseImplementation} and + * requiring the configuration's own lowercase spelling as a whole token + * excludes {@code debugImplementation}, {@code releaseImplementation} and * {@code testImplementation} without listing them, and cannot be defeated - * by a variant name nobody thought of. {@code api} is accepted alongside - * it because an api declaration is a real pin on the main variant; its - * variant forms are camel-cased in the same way.
+ * by a variant name nobody thought of. The other main-variant + * configurations are accepted alongside the one being written on; see + * {@link #MAIN_CONFIGURATIONS}. */ private static boolean declaresOnTheConstrainedConfiguration(String configuration, String line) { - if (line.contains(configuration)) { + if (declaresOn(configuration, line)) { return true; } - int at = line.indexOf("api"); + for (int i = 0; i < MAIN_CONFIGURATIONS.length; i++) { + if (declaresOn(MAIN_CONFIGURATIONS[i], line)) { + return true; + } + } + return false; + } + + /** + * The dependency configurations of the main variant, which is the one the + * constraints are written on. + * + *Every one of these reaches a classpath the {@code implementation} + * constraint also reaches, so a pin declared on any of them is the app + * managing the artifact. Getting the list short was a bug rather than a + * simplification: a {@code runtimeOnly} pin was read as unmanaged, and if + * it carried {@code strictly} the emitted 1.8.0 constraint did not override + * it but made the resolution fail outright -- worse than the override this + * class already tries to avoid.
+ * + *Their variant and test forms camel-case the configuration they derive + * from -- {@code testRuntimeOnly}, {@code debugCompileOnly}, + * {@code releaseApi} -- so matching the lowercase spelling as a whole token + * accepts the main ones and excludes the rest without listing any of them, + * whatever a variant happens to be called.
+ */ + private static final String[] MAIN_CONFIGURATIONS = { + "implementation", + "api", + "compileOnly", + "runtimeOnly", + "compile", + "runtime" + }; + + /** Whether this line declares on {@code configuration}, as a whole token. */ + private static boolean declaresOn(String configuration, String line) { + int at = line.indexOf(configuration); while (at >= 0) { - boolean startsToken = at == 0 || !Character.isLetterOrDigit(line.charAt(at - 1)); - int after = at + "api".length(); + boolean startsToken = at == 0 + || !Character.isLetterOrDigit(line.charAt(at - 1)); + int after = at + configuration.length(); boolean endsToken = after < line.length() - && (line.charAt(after) == ' ' || line.charAt(after) == '('); + && (line.charAt(after) == ' ' || line.charAt(after) == '(' + || line.charAt(after) == '\t'); if (startsToken && endsToken) { return true; } - at = line.indexOf("api", at + 1); + at = line.indexOf(configuration, at + 1); } return false; } @@ -477,7 +516,7 @@ private static String[] activeLines(String fragment) { } out.append(c); } - return statements(out.toString().split("\n")); + return statements(out.toString()); } /** @@ -505,53 +544,31 @@ private static String[] activeLines(String fragment) { * part that must not count. A bare {@code exclude} line truncates to * nothing and so is still not a pin. * + *A statement ends at a newline or at a semicolon, whichever comes + * first, and neither ends one inside parentheses or inside a string. The + * semicolon is not a nicety: this builder tells developers to separate + * {@code android.gradleDep} statements "with ';' or a newline", so a hint + * holding two declarations on one line is the documented shape. Splitting + * on newlines alone let the configuration token of the first statement pair + * with the coordinate of the second, which reads + * {@code implementation 'x'; debugImplementation platform('...kotlin-bom')} + * as a main-variant BOM and suppresses everything.
+ * *Joining stops at the end of the fragment: text left with parentheses * open is unbalanced Gradle, and rather than glue the remainder into one * long line -- which would make unrelated statements look like a single * declaration, and suppression is the direction that must never be reached * by accident -- its lines are kept as they were.
*/ - private static String[] statements(String[] lines) { - ListEvery one of these reaches a classpath the {@code implementation} - * constraint also reaches, so a pin declared on any of them is the app - * managing the artifact. Getting the list short was a bug rather than a - * simplification: a {@code runtimeOnly} pin was read as unmanaged, and if - * it carried {@code strictly} the emitted 1.8.0 constraint did not override - * it but made the resolution fail outright -- worse than the override this - * class already tries to avoid.
+ *Every one of these reaches the release RUNTIME classpath, which is the + * one {@code checkReleaseDuplicateClasses} reads and therefore the only one + * whose contents this class is trying to fix. {@code runtimeOnly} belongs + * here for exactly that reason.
+ * + *{@code compileOnly} does not, and putting it here was a mistake made + * by symmetry: a {@code compileOnly platform('...kotlin-bom')} is absent + * from the runtime graph, so treating it as the app managing that graph + * dropped the constraint from a classpath the app had not touched and left + * the duplicate in place. A compile-only declaration that would collide + * with the constraint is caught by the strict-version rule below instead, + * which is where that concern actually belongs.
* *Their variant and test forms camel-case the configuration they derive
* from -- {@code testRuntimeOnly}, {@code debugCompileOnly},
@@ -426,7 +441,6 @@ private static boolean declaresOnTheConstrainedConfiguration(String configuratio
private static final String[] MAIN_CONFIGURATIONS = {
"implementation",
"api",
- "compileOnly",
"runtimeOnly",
"compile",
"runtime"
@@ -603,15 +617,54 @@ private static String[] statements(String text) {
out.add(current.toString().replace('\n', ' '));
}
}
- List
Why the Kotlin Gradle plugin only sometimes excuses this. From - * 1.8.0 the plugin aligns the jdk variants itself, so the block would be a - * no-op and is skipped. An older plugin does not, and skipping there was a - * real bug: the versions were resolved with Gradle rather than reasoned - * about, and a project on the {@code android.useGradle8=false} path -- where - * this builder selects Kotlin 1.7.22 -- resolves like this:
+ *Why the Kotlin Gradle plugin does not excuse this. From 1.8.0 + * the plugin aligns the jdk variants itself, so it was tempting to skip the + * block whenever a new enough one was applied. That skip is gone, for two + * reasons that point the same way. It was never load-bearing: measured + * against a graph carrying billing 9.1.0 and appcompat 1.6.1, adding this + * block alongside plugin 1.9.22 and 1.8.22 produced byte-identical + * resolution, because the plugin's alignment already lands at or above this + * floor and a constraint never lowers a version. And it was not sound + * either -- the plugin's alignment can be turned off with + * {@code kotlin.stdlib.jdk.variants.version.alignment=false}, which this + * builder preserves out of a project's existing gradle.properties, so + * "a new enough plugin is applied" was never the same question as "the jdk + * variants are aligned".
+ * + *Deleting the skip answers both at once and takes with it the version + * parsing, the reading of {@code android.topDependency} and the hazard that + * a commented-out plugin declaration above an active one decided the + * outcome. An older plugin gets the block for the reason it always did: + * the 1.7 line ADDS {@code kotlin-stdlib-jdk8} at its own pre-merge version, + * so the class-bearing jar is guaranteed present and any dependency reaching + * a merged stdlib collides with it --
* ** plugin 1.7.22 alone stdlib 1.7.22 + jdk7/jdk8 1.7.22 no duplicate @@ -84,24 +99,14 @@ * the same, with this block stdlib 1.8.22 + jdk7/jdk8 1.8.0 fixed ** - *
Note the middle row is worse than a transitive accident: the 1.7.x - * plugin adds {@code kotlin-stdlib-jdk8} at its own version, so the - * older real jar is guaranteed present rather than merely possible, and any - * dependency that reaches a merged stdlib collides with it. Hence the test - * is the applied plugin's version, not whether a plugin is applied at all, - * and an unreadable version counts as "does not align" so the block is - * written rather than skipped.
- * - *The cost of that, stated plainly. On the same pre-1.8 plugin - * path, an app whose graph contains no merged stdlib (the first row above) - * did not need the block, and gets its stdlib family raised to + *
The cost of that, stated plainly. On that pre-1.8 plugin path, + * an app whose graph contains no merged stdlib (the first row above) did not + * need the block, and gets its stdlib family raised to * {@value #MERGED_STDLIB_FLOOR} anyway -- newer than the compiler in use, * which Kotlin warns about. That is deliberate. Gradle cannot express a - * constraint conditional on what another module resolved to, so the choice - * is between a warning in the case that did not need help and a failed build - * in the case that did, and a warning is the better of the two. Raising the - * builder's own pre-Gradle-8 Kotlin default would remove even that, and is a - * bigger change than this one should carry.
+ * constraint conditional on what another module resolved to, so the choice is + * between a warning in the case that did not need help and a failed build in + * the case that did, and a warning is the better of the two. * *Extracted into a pure static helper so it is unit-testable without a * Gradle run and so the BuildDaemon copy stays trivially diffable -- @@ -190,11 +195,6 @@ private KotlinStdlibAlignment() { * constraints on, {@code implementation} on any AndroidX project. The * caller passes the same name it uses for the rest of the block so a * legacy {@code compile} project stays consistent with itself. - * @param appliedKotlinPluginVersion the version of the Kotlin Gradle - * plugin this build applies, or null/empty when it applies none. Only - * {@value #MERGED_STDLIB_FLOOR} and newer align the jdk variants - * themselves; anything older, or anything this cannot read, is treated - * as not aligning and the block is written. * @param appGradleFragments the Gradle text the app itself contributed * ({@code gradleDependencies}, {@code android.gradleDep} and the like). * An artifact the app names there is left to the app; the Kotlin BOM @@ -202,10 +202,7 @@ private KotlinStdlibAlignment() { * @return the block, newline terminated, or {@code ""} */ public static String constraintsBlock(String configuration, - String appliedKotlinPluginVersion, String... appGradleFragments) { - if (alignsItsOwnJdkVariants(appliedKotlinPluginVersion)) { - return ""; - } + String... appGradleFragments) { if (configuration == null || configuration.trim().length() == 0) { return ""; } @@ -239,20 +236,6 @@ && atOrPastTheMerge(declaredVersion( return " constraints {\n" + out + " }\n"; } - /** - * Whether a Kotlin Gradle plugin of this version aligns the jdk stdlib - * variants on its own, making this class's block a no-op. - * - *
Answered from the version rather than from "is a plugin applied", - * because the two differ exactly where it matters. Unknown reads as - * false: a version that cannot be parsed -- an app declaring - * {@code kotlin-gradle-plugin:$kotlin_version} produces one -- must not - * silently switch the alignment off.
- */ - public static boolean alignsItsOwnJdkVariants(String kotlinPluginVersion) { - return atOrPastTheMerge(kotlinPluginVersion); - } - /** * Whether a Kotlin version is at or past the release that merged the jdk * artifacts away, and therefore aligns them wherever it is in force -- @@ -371,6 +354,24 @@ private static boolean declaresArtifactOnLine(String artifact, String configurat // it, so a strict pin ends the question regardless of which configuration // carries it. A strict version at or above the floor loses nothing by this: // it is already a shim. + // + // Reviewed twice as too broad -- a strict pin on debugImplementation or + // compileOnly does not manage releaseRuntimeClasspath, so suppressing the + // whole artifact leaves the release graph unaligned. That is true, and it is + // still the better of the two outcomes, because the constraint this block + // writes is NOT release-scoped: it is declared on `implementation`, which + // every variant inherits. There is no version of "constrain release but not + // debug" available from one implementation constraint. So the choice for an + // app with a strict pre-1.8 pin on a non-release configuration is: + // suppress -- release keeps a duplicate it already had before this change + // emit -- debug stops resolving, which it did fine before this change + // The second breaks a build that works today, and this class has taken the + // first everywhere else it has had to choose. Scoping the constraint to + // `releaseImplementation` would satisfy both, and is deliberately not done: + // naming a variant configuration that a given build type set may not have + // fails the whole script at evaluation, which is a far larger blast radius + // than the case it fixes. Revisit only with a project that actually has this + // shape. if (!line.contains("strictly") && !declaresOnTheConstrainedConfiguration(configuration, line)) { return false; @@ -464,26 +465,6 @@ private static boolean declaresOn(String configuration, String line) { return false; } - /** - * A Gradle fragment with its comments removed, for a caller that has to - * read a version out of it. - * - *Exposed because the builder parses {@code android.topDependency} for - * the Kotlin plugin version with a helper that takes the first bare - * substring match, so a commented-out declaration above an active one wins - * and decides the alignment. Same hazard as the one this class already - * guards against on its own fragments, reached through a different - * parser.
- */ - public static String activeText(String fragment) { - String[] lines = activeLines(fragment); - StringBuilder out = new StringBuilder(); - for (int i = 0; i < lines.length; i++) { - out.append(lines[i]).append('\n'); - } - return out.toString(); - } - /** * A fragment's lines with comments removed and exclusions dropped -- the * text that actually declares something. diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index 22f91791cd3..8bce5ca775a 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -34,14 +34,14 @@ * the alignment lands in the dependency graph of every AndroidX app, so the * cases that must produce nothing matter more than the one that must produce * something. The two that must NOT produce nothing -- - * {@link #aPreMergeKotlinPluginStillGetsTheAlignment()} and + * {@link #aKotlinPluginNoLongerExcusesTheBlock()} and * {@link #pinningOneJdkArtifactLeavesTheOtherConstrained()} -- are the ones * that caught a real over-suppression, so treat a change that makes either * pass vacuously as a regression. */ public class KotlinStdlibAlignmentTest { private static String block() { - return KotlinStdlibAlignment.constraintsBlock("implementation", null); + return KotlinStdlibAlignment.constraintsBlock("implementation"); } /** @@ -104,60 +104,22 @@ public void everyConstraintCarriesAReason() { } /** - * From 1.8.0 the Kotlin Gradle plugin aligns the jdk variants itself, so - * the block would be a no-op. - */ - @Test - public void skipsOnlyAKotlinPluginThatAlignsItself() { - check("".equals(KotlinStdlibAlignment.constraintsBlock( - "implementation", "1.8.0")), - "the release that starts aligning is skipped"); - check("".equals(KotlinStdlibAlignment.constraintsBlock( - "implementation", "1.9.22")), - "a newer plugin is skipped"); - check(KotlinStdlibAlignment.alignsItsOwnJdkVariants("2.0.0"), - "a major bump still aligns"); - check(KotlinStdlibAlignment.alignsItsOwnJdkVariants("1.9.22-RC2"), - "a qualifier does not hide an aligning version"); - } - - /** - * The case that made this a version test rather than an is-a-plugin-applied - * test. On the {@code android.useGradle8=false} path the builder selects - * Kotlin 1.7.22, which predates the merge and does not align. Worse, the - * 1.7 plugin ADDS {@code kotlin-stdlib-jdk8} at its own version, so the - * pre-merge real jar is guaranteed present; any dependency reaching a - * merged stdlib then collides with it. Measured with Gradle: plugin 1.7.22 - * plus billing 9.1.0 resolves kotlin-stdlib 1.8.22 beside jdk7/jdk8 - * 1.7.22, which is the duplicate. Skipping there shipped the bug. - */ - @Test - public void aPreMergeKotlinPluginStillGetsTheAlignment() { - check(KotlinStdlibAlignment.constraintsBlock("implementation", "1.7.22") - .contains("kotlin-stdlib-jdk8"), - "a pre-merge plugin still gets the alignment"); - check(!KotlinStdlibAlignment.alignsItsOwnJdkVariants("1.7.22"), - "1.7.22 does not align"); - check(!KotlinStdlibAlignment.alignsItsOwnJdkVariants("1.6.21"), - "1.6.21 does not align"); - } - - /** - * An app declaring {@code kotlin-gradle-plugin:$kotlin_version} parses to - * nothing. Unknown must read as "does not align" -- guessing the other way - * switches the fix off silently. + * A Kotlin plugin no longer excuses the block, whatever its version. + * + *Skipping for a 1.8+ plugin was never load-bearing -- measured against + * a graph carrying billing 9.1.0 and appcompat 1.6.1, adding this block + * alongside plugin 1.9.22 and 1.8.22 produced byte-identical resolution -- + * and it was not sound either, because the plugin's alignment can be + * turned off with kotlin.stdlib.jdk.variants.version.alignment=false, + * which this builder preserves out of a project's gradle.properties. + * Emitting unconditionally answers both, and takes the version parsing and + * the commented-plugin hazard with it.
*/ @Test - public void anUnreadablePluginVersionStillGetsTheAlignment() { - check(!KotlinStdlibAlignment.alignsItsOwnJdkVariants(""), - "no plugin does not align"); - check(!KotlinStdlibAlignment.alignsItsOwnJdkVariants(null), - "a null version does not align"); - check(!KotlinStdlibAlignment.alignsItsOwnJdkVariants("$kotlin_version"), - "a Gradle variable does not read as aligning"); - check(KotlinStdlibAlignment.constraintsBlock( - "implementation", "$kotlin_version").contains("kotlin-stdlib-jdk8"), - "an unreadable version still gets the alignment"); + public void aKotlinPluginNoLongerExcusesTheBlock() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation"); + check(out.contains("kotlin-stdlib-jdk7:1.8.0"), "jdk7 is aligned regardless"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), "jdk8 is aligned regardless"); } /** @@ -169,7 +131,7 @@ public void anUnreadablePluginVersionStillGetsTheAlignment() { @Test public void pinningOneJdkArtifactLeavesTheOtherConstrained() { String pinnedJdk7 = KotlinStdlibAlignment.constraintsBlock( - "implementation", null, + "implementation", " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.9.22'\n"); check(pinnedJdk7.contains("kotlin-stdlib-jdk8"), "pinning jdk7 leaves jdk8 constrained"); @@ -177,7 +139,7 @@ public void pinningOneJdkArtifactLeavesTheOtherConstrained() { "the artifact the app pinned is left to the app"); String pinnedJdk8 = KotlinStdlibAlignment.constraintsBlock( - "implementation", null, + "implementation", " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); check(pinnedJdk8.contains("kotlin-stdlib-jdk7"), "pinning jdk8 leaves jdk7 constrained"); @@ -185,7 +147,7 @@ public void pinningOneJdkArtifactLeavesTheOtherConstrained() { "the artifact the app pinned is left to the app"); String pinnedBoth = KotlinStdlibAlignment.constraintsBlock( - "implementation", null, + "implementation", " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.9.22'\n" + " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); check("".equals(pinnedBoth), @@ -207,11 +169,11 @@ public void pinningOneJdkArtifactLeavesTheOtherConstrained() { @Test public void emitsNothingWhenTheAppUsesTheKotlinBom() { check("".equals(KotlinStdlibAlignment.constraintsBlock( - "implementation", null, + "implementation", " implementation platform('org.jetbrains.kotlin:kotlin-bom:1.9.22')\n")), "an app using a merged-era Kotlin BOM is left alone"); check("".equals(KotlinStdlibAlignment.constraintsBlock( - "implementation", null, + "implementation", " implementation platform('org.jetbrains.kotlin:kotlin-bom:1.8.0')\n")), "the BOM at the merge itself is enough"); } @@ -224,7 +186,7 @@ public void emitsNothingWhenTheAppUsesTheKotlinBom() { @Test public void aPreMergeKotlinBomStillGetsTheAlignment() { String out = KotlinStdlibAlignment.constraintsBlock( - "implementation", null, + "implementation", " implementation platform('org.jetbrains.kotlin:kotlin-bom:1.7.22')\n"); check(out.contains("kotlin-stdlib-jdk7:1.8.0"), "a pre-merge BOM still gets jdk7 aligned"); @@ -239,7 +201,7 @@ public void aPreMergeKotlinBomStillGetsTheAlignment() { @Test public void anUnreadableBomVersionStillGetsTheAlignment() { String out = KotlinStdlibAlignment.constraintsBlock( - "implementation", null, + "implementation", " implementation platform(\"org.jetbrains.kotlin:kotlin-bom:$kotlinVersion\")\n"); check(out.contains("kotlin-stdlib-jdk8:1.8.0"), "an unreadable BOM version still gets the alignment"); @@ -294,44 +256,18 @@ public void theBuilderPassesEveryAppControlledDependencyFragment() throws Except */ @Test public void aCommentedOutDeclarationIsNotADeclaration() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", " // implementation platform('org.jetbrains.kotlin:kotlin-bom:1.9.22')\n" + " // implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); check(out.contains("kotlin-stdlib-jdk7:1.8.0"), "a commented-out BOM does not suppress"); check(out.contains("kotlin-stdlib-jdk8:1.8.0"), "a commented-out pin does not suppress"); - String blockComment = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String blockComment = KotlinStdlibAlignment.constraintsBlock("implementation", " /* implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22' */\n"); check(blockComment.contains("kotlin-stdlib-jdk8:1.8.0"), "a block-commented pin does not suppress"); } - /** - * activeText strips comments, and the builder has to use it on - * android.topDependency before the plugin version is parsed out of it. - * HealthManifestFragments takes the FIRST bare substring match, so a - * commented-out 1.8+ plugin above an active 1.7.x one is read as the - * applied version and the alignment is skipped for a build that needs it. - */ - @Test - public void aCommentedOutPluginIsNotTheAppliedPlugin() throws Exception { - String topDependency = - "// classpath 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.9.22'\n" - + "classpath 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.7.22'\n"; - check(KotlinStdlibAlignment.activeText(topDependency) - .indexOf("1.9.22") < 0, - "the commented plugin is gone from the active text"); - check(KotlinStdlibAlignment.activeText(topDependency) - .indexOf("1.7.22") >= 0, - "the active plugin survives"); - - byte[] bytes = java.nio.file.Files.readAllBytes(new java.io.File( - "src/main/java/com/codename1/builders/AndroidGradleBuilder.java").toPath()); - String builderSrc = new String(bytes, "UTF-8"); - check(builderSrc.contains("KotlinStdlibAlignment.activeText("), - "the builder strips comments before reading the plugin version"); - } - /** * A declaration on a variant or test configuration does not reach the one * the constraints are written on, so it cannot stand in for a pin. @@ -341,17 +277,17 @@ public void aCommentedOutPluginIsNotTheAppliedPlugin() throws Exception { */ @Test public void aVariantOnlyDeclarationDoesNotSuppress() { - String debugBom = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String debugBom = KotlinStdlibAlignment.constraintsBlock("implementation", " debugImplementation platform('org.jetbrains.kotlin:kotlin-bom:1.9.22')\n"); check(debugBom.contains("kotlin-stdlib-jdk8:1.8.0"), "a debug-only BOM does not suppress the main variant"); - String testPin = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String testPin = KotlinStdlibAlignment.constraintsBlock("implementation", " testImplementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); check(testPin.contains("kotlin-stdlib-jdk8:1.8.0"), "a test-only pin does not suppress the main variant"); - String releasePin = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String releasePin = KotlinStdlibAlignment.constraintsBlock("implementation", " releaseImplementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); check(releasePin.contains("kotlin-stdlib-jdk8:1.8.0"), "even a release-only pin is not the configuration being constrained"); @@ -369,7 +305,7 @@ public void aPinOnAnyMainConfigurationSuppresses() { String[] configurations = {"implementation", "api", "runtimeOnly", "compile", "runtime"}; for (String configuration : configurations) { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", " " + configuration + "('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22')\n"); check(!out.contains("kotlin-stdlib-jdk8:1.8.0"), @@ -388,7 +324,7 @@ public void aPinOnAnyMainConfigurationSuppresses() { */ @Test public void aCompileOnlyDeclarationDoesNotManageTheRuntimeGraph() { - String bom = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String bom = KotlinStdlibAlignment.constraintsBlock("implementation", " compileOnly platform('org.jetbrains.kotlin:kotlin-bom:1.9.22')\n"); check(bom.contains("kotlin-stdlib-jdk8:1.8.0"), "a compileOnly BOM does not align the runtime graph"); @@ -404,7 +340,7 @@ public void aCompileOnlyDeclarationDoesNotManageTheRuntimeGraph() { */ @Test public void aStrictPinIsHonouredOnAnyConfiguration() { - String releaseStrict = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String releaseStrict = KotlinStdlibAlignment.constraintsBlock("implementation", " releaseImplementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22') {\n" + " version { strictly '1.7.22' }\n" + " }\n"); @@ -412,7 +348,7 @@ public void aStrictPinIsHonouredOnAnyConfiguration() { "a strict release pin is left to the app"); String compileOnlyStrict = KotlinStdlibAlignment.constraintsBlock( - "implementation", null, + "implementation", " compileOnly('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22') " + "{ version { strictly '1.7.22' } }\n"); check(!compileOnlyStrict.contains("kotlin-stdlib-jdk8:1.8.0"), @@ -426,7 +362,7 @@ public void aStrictPinIsHonouredOnAnyConfiguration() { */ @Test public void anUnrelatedBlockDoesNotSwallowTheFragment() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", "dependencies {\n" + " implementation('com.example:thing:1.0') { version { strictly '1.0' } }\n" + " debugImplementation platform('org.jetbrains.kotlin:kotlin-bom:1.9.22')\n" @@ -444,7 +380,7 @@ public void theVariantFormsOfThoseConfigurationsStillDoNot() { String[] variants = {"testRuntimeOnly", "debugRuntimeOnly", "androidTestImplementation", "releaseCompileOnly", "debugApi", "testCompile"}; for (String variant : variants) { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", " " + variant + "('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22')\n"); check(out.contains("kotlin-stdlib-jdk8:1.8.0"), @@ -458,7 +394,7 @@ public void theVariantFormsOfThoseConfigurationsStillDoNot() { */ @Test public void anApiDeclarationCountsAsAPin() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", " api 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); check(!out.contains("kotlin-stdlib-jdk8:1.8.0"), "api pins jdk8"); check(out.contains("kotlin-stdlib-jdk7:1.8.0"), "and leaves jdk7 constrained"); @@ -472,7 +408,7 @@ public void anApiDeclarationCountsAsAPin() { */ @Test public void anExclusionIsNotAPin() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", " implementation('com.example:thing:1.0') {\n" + " exclude group: 'org.jetbrains.kotlin', module: 'kotlin-stdlib-jdk8'\n" + " }\n"); @@ -489,7 +425,7 @@ public void anExclusionIsNotAPin() { */ @Test public void aDeclarationSplitAcrossLinesIsStillAPin() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", " implementation(\n" + " 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'\n" + " )\n"); @@ -506,7 +442,7 @@ public void aDeclarationSplitAcrossLinesIsStillAPin() { */ @Test public void anInlineExclusionDoesNotCancelTheDeclaration() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", " implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22') " + "{ exclude group: 'com.example', module: 'thing' }\n"); check(!out.contains("kotlin-stdlib-jdk8:1.8.0"), @@ -519,7 +455,7 @@ public void anInlineExclusionDoesNotCancelTheDeclaration() { */ @Test public void aStandaloneExclusionIsStillNotAPin() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", " implementation('com.example:thing:1.0') {\n" + " exclude group: 'org.jetbrains.kotlin', module: 'kotlin-stdlib-jdk8'\n" + " }\n"); @@ -537,7 +473,7 @@ public void aStandaloneExclusionIsStillNotAPin() { */ @Test public void aSemicolonEndsAStatement() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", " implementation 'com.android.billingclient:billing:9.1.0'; " + "debugImplementation platform('org.jetbrains.kotlin:kotlin-bom:1.9.22')\n"); check(out.contains("kotlin-stdlib-jdk8:1.8.0"), @@ -546,7 +482,7 @@ public void aSemicolonEndsAStatement() { // The same shape where the pin IS on the main variant still suppresses, so // the split did not simply stop every semicolon-separated value from working. - String pinned = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String pinned = KotlinStdlibAlignment.constraintsBlock("implementation", " implementation 'com.android.billingclient:billing:9.1.0'; " + "implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); check(!pinned.contains("kotlin-stdlib-jdk8:1.8.0"), @@ -558,14 +494,14 @@ public void aSemicolonEndsAStatement() { */ @Test public void aSemicolonInsideAStringOrParensIsNotASeparator() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", " implementation(\n" + " 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'\n" + " )\n"); check(!out.contains("kotlin-stdlib-jdk8:1.8.0"), "a wrapped declaration still pins"); - String quoted = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String quoted = KotlinStdlibAlignment.constraintsBlock("implementation", " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22' " + "// note; with a semicolon\n"); check(!quoted.contains("kotlin-stdlib-jdk8:1.8.0"), @@ -580,7 +516,7 @@ public void aSemicolonInsideAStringOrParensIsNotASeparator() { */ @Test public void unbalancedParenthesesDoNotGlueStatementsTogether() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", " implementation(\n" + " testImplementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'\n"); check(out.contains("kotlin-stdlib-jdk8:1.8.0"), @@ -593,7 +529,7 @@ public void unbalancedParenthesesDoNotGlueStatementsTogether() { */ @Test public void theMapFormCountsAsAPin() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", " implementation group: 'org.jetbrains.kotlin', " + "name: 'kotlin-stdlib-jdk8', version: '1.9.22'\n"); check(!out.contains("kotlin-stdlib-jdk8:1.8.0"), "the map form pins jdk8"); @@ -607,7 +543,7 @@ public void theMapFormCountsAsAPin() { */ @Test public void aUrlIsNotAComment() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", " maven { url 'https://example.com/repo' }\n" + " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); check(!out.contains("kotlin-stdlib-jdk8:1.8.0"), @@ -621,7 +557,7 @@ public void aUrlIsNotAComment() { */ @Test public void ignoresEmptyAndNullFragments() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", "", null, " implementation 'androidx.appcompat:appcompat:1.6.1'\n"); check(out.contains("kotlin-stdlib-jdk8"), "an empty or absent hint is not a pin"); @@ -635,7 +571,7 @@ public void ignoresEmptyAndNullFragments() { */ @Test public void anUnrelatedKotlinDependencyIsNotAPin() { - String out = KotlinStdlibAlignment.constraintsBlock("implementation", null, + String out = KotlinStdlibAlignment.constraintsBlock("implementation", " implementation 'org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4'\n"); check(out.contains("kotlin-stdlib-jdk8"), "a coroutines dependency does not switch the alignment off"); @@ -648,7 +584,7 @@ public void anUnrelatedKotlinDependencyIsNotAPin() { */ @Test public void usesTheConfigurationItWasGiven() { - String out = KotlinStdlibAlignment.constraintsBlock("compile", null); + String out = KotlinStdlibAlignment.constraintsBlock("compile"); check(out.contains("compile('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.0')"), "the caller's configuration is used"); check(!out.contains("implementation("), @@ -657,9 +593,9 @@ public void usesTheConfigurationItWasGiven() { @Test public void emitsNothingWithoutAConfiguration() { - check("".equals(KotlinStdlibAlignment.constraintsBlock(null, null)), + check("".equals(KotlinStdlibAlignment.constraintsBlock(null)), "a null configuration writes nothing"); - check("".equals(KotlinStdlibAlignment.constraintsBlock(" ", null)), + check("".equals(KotlinStdlibAlignment.constraintsBlock(" ")), "a blank configuration writes nothing"); } From 27f413fc077c01f4f1be938f5a0381d659b083fe Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 15:11:31 +0300 Subject: [PATCH 12/94] Delete the exclusion cut, and let the comment stripper see strings Two findings, and both fixes remove code rather than adding a case to it. The exclusion cut is gone. It was needed while a declaration was recognised by the artifact name appearing anywhere, and became unnecessary the moment a declaration had to be spelled as one: an exclusion writes `group: '...', module: 'kotlin-stdlib-jdk8'` and never the colon-joined coordinate or the `name:` map form the check looks for, so it cannot match a declaration in the first place. All three exclusion cases still pass with the cut deleted, which is what says it was redundant rather than load-bearing. It was also actively harmful, which is what the review found. Cutting from `exclude` to the end of the statement discarded everything after it, so an exclusion written before a `version { strictly '1.7.22' } }` block took that block with it -- and losing a strict marker is what turns this class's constraint into a failed resolution instead of an override. The comment stripper now tracks quoted strings, which the statement scanner beside it already did for parentheses and semicolons. A `/*` inside a quoted value used to open a block comment that swallowed the rest of the fragment, including a strict pin, with the same consequence. Tracking quotes covers that and subsumes the narrower rule it replaces: `//` was previously spared only when it followed a colon, which was a way of protecting `url 'https://...'` without noticing the general case. A `//` inside any string is now safe, in either quote style, and the URL case keeps its own test. Three new cases, each checked by putting the old behaviour back. Co-Authored-By: Claude Opus 5 (1M context)A line comment is only a comment when the {@code //} does not follow - * a colon: {@code maven { url 'https://...' }} is an ordinary declaration - * that a naive strip would cut in half, and these fragments really do - * carry repository URLs.
+ *Comment delimiters are only delimiters outside a string, which is the + * same rule the statement scanner already applied to parentheses and + * semicolons and which this had been missing. It matters in both + * directions: {@code maven { url 'https://...' }} is an ordinary + * declaration that a naive strip cuts in half, and a {@code /*} inside a + * string used to open a block comment that swallowed the rest of the + * fragment -- including, in the case that found this, an explicit strict + * pin whose loss turns this class's constraint into a failed resolution. + * Tracking quotes covers both, and replaces the narrower rule that only + * spared a {@code //} following a colon.
* *Regrouping into statements is {@link #statements}; this method only * removes the comments.
@@ -483,6 +489,7 @@ private static String[] activeLines(String fragment) { } StringBuilder out = new StringBuilder(); boolean inBlockComment = false; + char quote = 0; for (int i = 0; i < fragment.length(); i++) { char c = fragment.charAt(i); if (inBlockComment) { @@ -494,6 +501,21 @@ private static String[] activeLines(String fragment) { } continue; } + if (quote != 0) { + out.append(c); + if (c == '\\' && i + 1 < fragment.length()) { + out.append(fragment.charAt(i + 1)); + i++; + } else if (c == quote) { + quote = 0; + } + continue; + } + if (c == '\'' || c == '"') { + quote = c; + out.append(c); + continue; + } if (c == '/' && i + 1 < fragment.length()) { char next = fragment.charAt(i + 1); if (next == '*') { @@ -501,7 +523,7 @@ private static String[] activeLines(String fragment) { i++; continue; } - if (next == '/' && (i == 0 || fragment.charAt(i - 1) != ':')) { + if (next == '/') { while (i < fragment.length() && fragment.charAt(i) != '\n') { i++; } @@ -533,11 +555,19 @@ private static String[] activeLines(String fragment) { * for containing "exclude" * * - *So a line whose parentheses are still open is joined to the next, and - * a statement is truncated at {@code exclude} rather than discarded -- what - * precedes the exclusion is the declaration, and what follows it is the - * part that must not count. A bare {@code exclude} line truncates to - * nothing and so is still not a pin.
+ *So a line whose parentheses are still open is joined to the next. + * Exclusions are left alone: they used to be cut out here, which was + * needed while a declaration was recognised by the artifact name appearing + * anywhere, and became both unnecessary and harmful once a declaration had + * to be spelled as one. Unnecessary, because an exclusion writes + * {@code group: '...', module: 'kotlin-stdlib-jdk8'} and never the + * colon-joined coordinate or the {@code name:} map form the declaration + * check looks for, so it cannot match one. Harmful, because cutting from + * {@code exclude} to the end of the statement also threw away anything + * after it -- an exclusion written before a + * {@code version { strictly '1.7.22' } }} block took that block with it, + * and losing the strict marker is what turns this class's constraint into + * a failed resolution.
* *A statement ends at a newline or at a semicolon, whichever comes
* first, and neither ends one inside parentheses or inside a string. The
@@ -616,13 +646,7 @@ private static String[] statements(String text) {
}
merged.add(statement);
}
- List
Why nothing else excuses the block either. A Kotlin BOM used to + * suppress it, on the reasoning that a BOM manages the whole + * {@code org.jetbrains.kotlin} group. That went the way of the plugin check + * and for the same measured reason: against a graph carrying billing 9.1.0 + * and appcompat 1.6.1, adding this block alongside {@code kotlin-bom:1.9.22} + * produced byte-identical resolution, because a BOM's constraints are not + * strict and the higher version simply wins. Alongside + * {@code kotlin-bom:1.7.22} it is not merely harmless but necessary -- a + * pre-merge BOM raises the jdk artifacts and cannot pull + * {@code kotlin-stdlib} back down, which is the duplicate. Removing the case + * also removes the question of whether a BOM declared inside an {@code if} + * block is in force, which no amount of reading the text can answer.
+ * *Why a constraint and not a force. A constraint raises a version * and never lowers one, and never pulls a module into a graph that does not * already contain it. An app with no Kotlin anywhere is therefore completely @@ -151,35 +164,6 @@ public class KotlinStdlibAlignment { "kotlin-stdlib-jdk8" }; - /** - * The marker that can suppress the whole block rather than one artifact. - * A BOM manages every module in the {@code org.jetbrains.kotlin} group, - * jdk7 and jdk8 included, so a new enough one answers the question for - * both artifacts at once. - * - *
Only a new enough one. A BOM raises the jdk artifacts but - * cannot pull {@code kotlin-stdlib} back down, because a platform - * contributes constraints and the highest version still wins. So a - * pre-merge BOM leaves exactly the arrangement this class exists to - * prevent -- measured, with the same graph as the class comment's - * table:
- * - *- * no BOM stdlib 1.8.22 + jdk7/jdk8 1.6.21 duplicate - * kotlin-bom 1.7.22 stdlib 1.8.22 + jdk7/jdk8 1.7.22 STILL a duplicate - * kotlin-bom 1.9.22 all 1.9.22, jdk artifacts shims safe - *- * - *
The BOM is therefore tested by version, exactly like the Kotlin - * Gradle plugin above it, and for the same reason: presence is not - * alignment.
- */ - private static final String KOTLIN_BOM = "kotlin-bom"; - - /** The coordinate a BOM's version is read from. */ - private static final String KOTLIN_BOM_COORDINATE = - "org.jetbrains.kotlin:kotlin-bom:"; - /** The group every artifact this class reasons about belongs to. */ private static final String KOTLIN_GROUP = "org.jetbrains.kotlin"; @@ -207,11 +191,6 @@ public static String constraintsBlock(String configuration, return ""; } String config = configuration.trim(); - if (declaresArtifact(KOTLIN_BOM, config, appGradleFragments) - && atOrPastTheMerge(declaredVersion( - KOTLIN_BOM_COORDINATE, config, appGradleFragments))) { - return ""; - } // "because" is not decoration: it is what `gradle dependencyInsight` prints // next to the raised version, and this constraint is otherwise unattributable // to anything in the developer's project. @@ -236,72 +215,6 @@ && atOrPastTheMerge(declaredVersion( return " constraints {\n" + out + " }\n"; } - /** - * Whether a Kotlin version is at or past the release that merged the jdk - * artifacts away, and therefore aligns them wherever it is in force -- - * as the Gradle plugin's version or as a BOM's. - * - *Unknown reads as false everywhere it is used. A version that cannot - * be parsed -- {@code kotlin-gradle-plugin:$kotlin_version} and - * {@code kotlin-bom:$kotlinVersion} both produce one -- must not silently - * switch the alignment off.
- */ - private static boolean atOrPastTheMerge(String kotlinVersion) { - if (kotlinVersion == null) { - return false; - } - // Shared with the Health Connect floor check rather than parsed again here: - // it already drops a qualifier, which rounds a prerelease up to its release - // and is the forgiving direction for a floor. - String numeric = HealthManifestFragments.numericVersionPrefix( - kotlinVersion.trim()); - if (numeric == null) { - return false; - } - return compareVersions(numeric, MERGED_STDLIB_FLOOR) >= 0; - } - - /** - * The version an app's own Gradle text declares immediately after - * {@code coordinate}, or null when it declares none there or writes one - * this cannot read -- a Gradle variable rather than a literal. - */ - private static String declaredVersion(String coordinate, String configuration, - String[] appGradleFragments) { - if (appGradleFragments == null) { - return null; - } - for (int i = 0; i < appGradleFragments.length; i++) { - // The same active text the declaration check reads, so a commented-out - // BOM cannot supply the version that suppresses the block. - String[] lines = activeLines(appGradleFragments[i]); - for (int j = 0; j < lines.length; j++) { - String fragment = lines[j]; - // Same configuration filter as the declaration check, so a debug-only - // BOM cannot supply the version that suppresses the main variant's - // constraints. - if (!declaresOnTheConstrainedConfiguration(configuration, fragment)) { - continue; - } - int at = fragment.indexOf(coordinate); - if (at < 0) { - continue; - } - int from = at + coordinate.length(); - int to = from; - while (to < fragment.length() - && "0123456789.".indexOf(fragment.charAt(to)) >= 0) { - to++; - } - while (to > from && fragment.charAt(to - 1) == '.') { - to--; - } - return to > from ? fragment.substring(from, to) : null; - } - } - return null; - } - /** * Whether the app actively declares this {@code org.jetbrains.kotlin} * artifact, rather than merely mentioning its name somewhere in a Gradle @@ -594,7 +507,15 @@ private static String[] statements(String text) { char c = text.charAt(i); if (quote != 0) { current.append(c); - if (c == quote) { + // Escapes, because the comment stripper beside this already handles + // them: 'can\'t' otherwise closes the string on the apostrophe it is + // escaping, and every following newline is read as being inside a + // string rather than ending a statement -- which merges statements + // that must stay apart. + if (c == '\\' && i + 1 < text.length()) { + current.append(text.charAt(i + 1)); + i++; + } else if (c == quote) { quote = 0; } continue; @@ -670,26 +591,4 @@ private static int braceBalance(String statement) { return depth; } - /** Numeric dotted version compare; a missing segment counts as zero. */ - private static int compareVersions(String left, String right) { - String[] l = left.split("\\."); - String[] r = right.split("\\."); - int len = Math.max(l.length, r.length); - for (int i = 0; i < len; i++) { - int a = i < l.length ? parse(l[i]) : 0; - int b = i < r.length ? parse(r[i]) : 0; - if (a != b) { - return a < b ? -1 : 1; - } - } - return 0; - } - - private static int parse(String segment) { - try { - return Integer.parseInt(segment); - } catch (NumberFormatException notANumber) { - return 0; - } - } } diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index dbea397cdb8..b5f58f1b027 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -154,59 +154,6 @@ public void pinningOneJdkArtifactLeavesTheOtherConstrained() { "an app managing both gets no block at all, not an empty one"); } - /** - * A BOM manages the whole {@code org.jetbrains.kotlin} group, jdk7 and - * jdk8 included, so a new enough one is the single marker that suppresses - * both constraints. - * - *New enough is the whole point. A BOM raises the jdk artifacts but - * cannot pull {@code kotlin-stdlib} down -- a platform contributes - * constraints and the highest version still wins -- so a pre-merge BOM - * leaves a merged stdlib beside class-bearing jdk jars, which is the - * duplicate. Measured: kotlin-bom 1.7.22 against a graph wanting stdlib - * 1.8.22 resolves jdk7/jdk8 to 1.7.22, still class-bearing.
- */ - @Test - public void emitsNothingWhenTheAppUsesTheKotlinBom() { - check("".equals(KotlinStdlibAlignment.constraintsBlock( - "implementation", - " implementation platform('org.jetbrains.kotlin:kotlin-bom:1.9.22')\n")), - "an app using a merged-era Kotlin BOM is left alone"); - check("".equals(KotlinStdlibAlignment.constraintsBlock( - "implementation", - " implementation platform('org.jetbrains.kotlin:kotlin-bom:1.8.0')\n")), - "the BOM at the merge itself is enough"); - } - - /** - * The counterpart, and the reason the BOM is read by version rather than - * by presence: a pre-merge BOM does not make the graph safe, so it must - * not switch the block off. - */ - @Test - public void aPreMergeKotlinBomStillGetsTheAlignment() { - String out = KotlinStdlibAlignment.constraintsBlock( - "implementation", - " implementation platform('org.jetbrains.kotlin:kotlin-bom:1.7.22')\n"); - check(out.contains("kotlin-stdlib-jdk7:1.8.0"), - "a pre-merge BOM still gets jdk7 aligned"); - check(out.contains("kotlin-stdlib-jdk8:1.8.0"), - "a pre-merge BOM still gets jdk8 aligned"); - } - - /** - * A BOM whose version is a Gradle variable reads as unknown, and unknown - * must not suppress -- the same fail-safe the plugin version gets. - */ - @Test - public void anUnreadableBomVersionStillGetsTheAlignment() { - String out = KotlinStdlibAlignment.constraintsBlock( - "implementation", - " implementation platform(\"org.jetbrains.kotlin:kotlin-bom:$kotlinVersion\")\n"); - check(out.contains("kotlin-stdlib-jdk8:1.8.0"), - "an unreadable BOM version still gets the alignment"); - } - /** * The builder has to hand over every app-controlled fragment that reaches * the generated dependencies block, not the ones that came to mind. @@ -596,6 +543,63 @@ public void aUrlIsNotAComment() { "the pin after a URL line is still seen"); } + /** + * A Kotlin BOM no longer excuses the block either, at any version. + * + *Measured against a graph carrying billing 9.1.0 and appcompat 1.6.1: + * adding this block alongside kotlin-bom 1.9.22 gives byte-identical + * resolution, because a BOM's constraints are not strict and the higher + * version wins; alongside kotlin-bom 1.7.22 it is not merely harmless but + * necessary, since a pre-merge BOM raises the jdk artifacts and cannot + * pull kotlin-stdlib back down. Suppressing on a BOM was cosmetic where it + * fired and wrong where it did not.
+ */ + @Test + public void aKotlinBomNoLongerExcusesTheBlock() { + String modern = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation platform('org.jetbrains.kotlin:kotlin-bom:1.9.22')\n"); + check(modern.contains("kotlin-stdlib-jdk8:1.8.0"), + "a modern BOM does not suppress, and does not need to"); + + String preMerge = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation platform('org.jetbrains.kotlin:kotlin-bom:1.7.22')\n"); + check(preMerge.contains("kotlin-stdlib-jdk8:1.8.0"), + "a pre-merge BOM still gets the alignment it needs"); + } + + /** + * And the case that removed the feature rather than patching it: a BOM + * declared inside a condition cannot be known to be in force by reading + * the text, so no reading of it decides anything any more. + */ + @Test + public void aConditionalBomDoesNotDecideAnything() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " if (project.hasProperty('useKotlinBom')) {\n" + + " implementation platform('org.jetbrains.kotlin:kotlin-bom:1.9.22')\n" + + " }\n"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), + "a conditional BOM leaves the alignment in place"); + } + + /** + * An escaped quote does not end a string. The statement scanner missed + * this while the comment stripper beside it handled it, so a string + * escaping its own apostrophe closed early and every following newline + * read as being inside a string -- merging statements that must stay + * apart, which lets a main-variant configuration token pair with a + * debug-only coordinate. + */ + @Test + public void anEscapedQuoteDoesNotEndAString() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " def marker = 'can\\'t'\n" + + " implementation 'com.android.billingclient:billing:9.1.0'\n" + + " debugImplementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), + "the debug-only pin does not borrow the main statement's configuration"); + } + /** * The fragments arrive straight from build hints, so an unset hint shows * up as an empty string and an absent one can be null. Neither is a From 3d40900bda6c242120febb754456be9627fa3a96 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 15:45:20 +0300 Subject: [PATCH 14/94] Tell Gradle syntax from English, and let a map entry breathe Three findings, all in the family this class keeps meeting: text that looks like syntax is not syntax, and syntax spelled differently is still syntax. `strictly` is now detected as a call rather than as a substring. A reason reading `because 'not strictly required outside debug'` is prose, and reading it as a version constraint let a variant-only dependency switch the alignment off for the release build -- the unsafe direction, and the one this class has been careful about everywhere else. The discriminator is the one already used for comment delimiters and statement separators: inside a string it is prose, outside it is syntax. The real call still suppresses, and has its own check beside the prose one so a future tightening cannot quietly disarm it. A Groovy map entry may have whitespace around its colon. `name : 'x'` is as valid as `name: 'x'`, and the exact substring match missed it -- which matters because the same declaration can carry a strict version, so missing it turns this block's constraint into a failed resolution rather than an override. Parsed properly now, in either quote style, and a different Kotlin artifact in the same shape still does not count. A quoted configuration name counts too, for Gradle's `dependencies.add("runtimeOnly", "group:artifact:version")` spelling: the token ends at a quote there rather than at a space or a parenthesis. Safe to accept, because a quoted configuration name only decides anything on a statement that also carries the artifact coordinate -- a statement carrying both is a declaration however it is spelled, and one carrying only the name is not. Five new cases. Each behaviour was checked by putting the old one back, and three of the five pin the directions these fixes could have gone too far. Co-Authored-By: Claude Opus 5 (1M context){@code because 'not strictly required outside debug'} is a reason + * string, not a version constraint, and reading it as one let a + * variant-only dependency switch the alignment off for the release build. + * The discriminator is the one already used for comment delimiters and + * statement separators: inside a string it is prose, outside it is + * syntax.
+ */ + private static boolean callsStrictly(String statement) { + char quote = 0; + for (int i = 0; i < statement.length(); i++) { + char c = statement.charAt(i); + if (quote != 0) { + if (c == '\\' && i + 1 < statement.length()) { + i++; + } else if (c == quote) { + quote = 0; + } + continue; + } + if (c == '\'' || c == '"') { + quote = c; + continue; + } + if (statement.startsWith(STRICTLY, i)) { + boolean startsToken = i == 0 + || !Character.isLetterOrDigit(statement.charAt(i - 1)); + int after = i + STRICTLY.length(); + boolean endsToken = after < statement.length() + && (statement.charAt(after) == ' ' + || statement.charAt(after) == '(' + || statement.charAt(after) == '\t'); + if (startsToken && endsToken) { + return true; + } + } + } + return false; + } + + private static final String STRICTLY = "strictly"; + + /** + * Whether the statement carries the Groovy map entry + * {@code key: 'value'}, with whatever spacing the author used. + * + *{@code name : 'kotlin-stdlib-jdk8'} is as valid as + * {@code name: 'kotlin-stdlib-jdk8'}, and matching the exact substring + * missed it -- which matters because the same declaration can carry a + * strict version, and missing it turns this class's constraint into a + * failed resolution.
+ */ + private static boolean declaresMapEntry(String line, String key, String value) { + int at = line.indexOf(key); + while (at >= 0) { + boolean startsToken = at == 0 + || !Character.isLetterOrDigit(line.charAt(at - 1)); + if (startsToken) { + int i = skipBlanks(line, at + key.length()); + if (i < line.length() && line.charAt(i) == ':') { + i = skipBlanks(line, i + 1); + if (i < line.length() + && (line.charAt(i) == '\'' || line.charAt(i) == '"')) { + char q = line.charAt(i); + int end = line.indexOf(q, i + 1); + if (end > i && line.substring(i + 1, end).equals(value)) { + return true; + } + } + } + } + at = line.indexOf(key, at + 1); + } + return false; + } + + private static int skipBlanks(String line, int from) { + int i = from; + while (i < line.length() && (line.charAt(i) == ' ' || line.charAt(i) == '\t')) { + i++; + } + return i; } /** @@ -367,9 +452,16 @@ private static boolean declaresOn(String configuration, String line) { boolean startsToken = at == 0 || !Character.isLetterOrDigit(line.charAt(at - 1)); int after = at + configuration.length(); + // A closing quote ends the token too, for the map-free + // dependencies.add("runtimeOnly", "group:artifact:version") spelling. + // Safe to accept: a quoted configuration name only decides anything on a + // statement that also carries the artifact coordinate, and a statement + // carrying both is a declaration however it is spelled. boolean endsToken = after < line.length() && (line.charAt(after) == ' ' || line.charAt(after) == '(' - || line.charAt(after) == '\t'); + || line.charAt(after) == '\t' + || line.charAt(after) == '"' + || line.charAt(after) == '\''); if (startsToken && endsToken) { return true; } diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index b5f58f1b027..ce4f3b2583b 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -325,6 +325,56 @@ public void anExclusionBeforeTheVersionBlockDoesNotHideTheStrictPin() { "and the same written across lines"); } + /** + * The English word is not the Gradle call. A reason string reading + * "not strictly required outside debug" is prose, and reading it as a + * strict version let a variant-only dependency switch the alignment off + * for the release build -- the unsafe direction. + */ + @Test + public void theWordStrictlyInsideAStringIsNotAStrictPin() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " debugImplementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22') " + + "{ because 'not strictly required outside debug' }\n"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), + "a reason mentioning the word does not suppress the release constraint"); + + // and the real call still does, so the tightening did not disarm it + String real = KotlinStdlibAlignment.constraintsBlock("implementation", + " debugImplementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22') " + + "{ version { strictly '1.7.22' } }\n"); + check(!real.contains("kotlin-stdlib-jdk8:1.8.0"), + "an actual strict call is still honoured"); + } + + /** + * Groovy allows whitespace around a map entry's colon, and the exact + * substring match missed it. It matters because the same declaration can + * carry a strict version, and missing it turns the constraint into a + * failed resolution rather than an override. + */ + @Test + public void aMapEntryMayHaveSpaceAroundItsColon() { + String spaced = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation(group : 'org.jetbrains.kotlin', " + + "name : 'kotlin-stdlib-jdk8', version : '1.7.22')\n"); + check(!spaced.contains("kotlin-stdlib-jdk8:1.8.0"), + "a spaced map entry still pins jdk8"); + + String doubleQuoted = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation(group: \"org.jetbrains.kotlin\", " + + "name:\"kotlin-stdlib-jdk8\", version: \"1.7.22\")\n"); + check(!doubleQuoted.contains("kotlin-stdlib-jdk8:1.8.0"), + "and so does an unspaced double-quoted one"); + + // A different artifact in the same shape must still not count. + String other = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation(group : 'org.jetbrains.kotlin', " + + "name : 'kotlin-reflect', version : '1.9.22')\n"); + check(other.contains("kotlin-stdlib-jdk8:1.8.0"), + "naming a different Kotlin artifact does not pin jdk8"); + } + /** * The block absorbed for that check belongs to the declaration that opened * it and no further. A dependencies or android block must not swallow the @@ -341,6 +391,37 @@ public void anUnrelatedBlockDoesNotSwallowTheFragment() { "an unrelated strict block does not suppress, and the debug BOM still does not"); } + /** + * The quoted spelling counts as well. Gradle's + * {@code dependencies.add("runtimeOnly", "group:artifact:version")} names + * the configuration as a string, so the token ends at a quote rather than + * a space or a parenthesis, and the escape hatch has to recognise it for + * the same reason it recognises the others. + */ + @Test + public void theQuotedAddSpellingCountsAsAPin() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " dependencies.add(\"runtimeOnly\", " + + "\"org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22\")\n"); + check(!out.contains("kotlin-stdlib-jdk8:1.8.0"), + "a quoted configuration name still pins jdk8"); + check(out.contains("kotlin-stdlib-jdk7:1.8.0"), + "and leaves jdk7 constrained"); + } + + /** + * A quoted configuration name on its own decides nothing, because it takes + * the artifact coordinate on the same statement to make a declaration. + */ + @Test + public void aQuotedConfigurationNameAloneIsNotAPin() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " def cfg = \"runtimeOnly\"\n" + + " implementation 'com.example:thing:1.0'\n"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), + "naming a configuration in a string does not suppress anything"); + } + /** * And their variant and test forms still do not, which is the property the * whole-token lowercase match buys without listing a single variant name. From 3b26c9d73dd8a15489484876c203b00a49d1ac9b Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:22:30 +0300 Subject: [PATCH 15/94] Honour a strict pin on the merged stdlib, and say what is not being fixed One finding taken, two not, and a decision left where it belongs. A strict pin on kotlin-stdlib ITSELF blocks both shims, not one. The shim at this floor depends on kotlin-stdlib at the same floor, so an app strictly holding the base library below it cannot resolve either constraint -- and the pre-merge family it is holding had no duplicate to begin with. Constraining there converts a working build into Could not resolve org.jetbrains.kotlin:kotlin-stdlib-jdk8:{strictly 1.7.22} which is the one outcome this class must never produce. The base match is exact, because kotlin-stdlib is a prefix of kotlin-stdlib-jdk8 and a loose match would read every shim declaration as a pin on the base library and disable the block entirely; there is a case for that, and one confirming a strict pin at or above the floor still gets the constraints, since a shim requiring 1.8.0 is satisfied by a strict 1.9.22. Not taken: gating on the constraints DSL instead of android.useAndroidX. The premise is that the builder selects `implementation` for every Gradle 6 build, and it does not -- `compile` becomes "implementation" only when useAndroidX or the aar implementation flag is set, so a useAndroidX=false build would take this block on the legacy `compile` configuration. The failing case it is meant to protect also needs a modern AndroidX dependency inside a project with AndroidX switched off, which AGP refuses for its own reasons first. The reasoning is on the gate now rather than only in a thread. Not taken here, and stated rather than quietly ignored: a declaration inside `if (project.hasProperty('x'))` is treated as present. Whether it is in force is decided by Gradle at evaluation time and cannot be read out of the text, so the choice is between honouring the documented promise at the cost of leaving a duplicate the app already had, and suppressing only on a strict version. The second is the better rule -- it is the only declaration a constraint cannot coexist with, and it would take the configuration filtering, the spelling variants and this hazard with it -- but it changes what the build hint's documentation promises, so it is the project's call and not something to slip in under a review thread. It was written and reverted here rather than guessed at: twelve existing cases assert the current behaviour, which is the measure of how much of this file that decision governs. Co-Authored-By: Claude Opus 5 (1M context)The artifact has to be matched exactly. {@code kotlin-stdlib} is a + * prefix of {@code kotlin-stdlib-jdk8}, so a loose match would read every + * shim declaration as a pin on the base library and switch the whole block + * off. The character after the coordinate decides it: a colon starts the + * version and a quote ends the coordinate, while anything else -- a + * hyphen above all -- means this is a longer artifact name.
+ * + *An unreadable strict version counts as below the floor, because the + * failure it guards against cannot be worked around by the app while the + * duplicate class it risks instead can.
+ */ + private static boolean strictlyPinsBaseStdlibBelowTheFloor(String[] appGradleFragments) { + if (appGradleFragments == null) { + return false; + } + for (int i = 0; i < appGradleFragments.length; i++) { + String[] lines = activeLines(appGradleFragments[i]); + for (int j = 0; j < lines.length; j++) { + if (!callsStrictly(lines[j]) || !namesBaseStdlib(lines[j])) { + continue; + } + String version = strictVersionIn(lines[j]); + if (version == null + || compareVersions(version, MERGED_STDLIB_FLOOR) < 0) { + return true; + } + } + } + return false; + } + + /** Whether the statement names {@code kotlin-stdlib} and not a longer name. */ + private static boolean namesBaseStdlib(String line) { + String coordinate = KOTLIN_GROUP + ":" + BASE_STDLIB; + int at = line.indexOf(coordinate); + while (at >= 0) { + int after = at + coordinate.length(); + if (after < line.length() + && (line.charAt(after) == ':' || line.charAt(after) == '\'' + || line.charAt(after) == '"')) { + return true; + } + at = line.indexOf(coordinate, at + 1); + } + return line.contains(KOTLIN_GROUP) && declaresMapEntry(line, "name", BASE_STDLIB); + } + + /** The version inside this statement's {@code strictly} call, or null. */ + private static String strictVersionIn(String statement) { + int at = statement.indexOf(STRICTLY); + while (at >= 0) { + int i = skipBlanks(statement, at + STRICTLY.length()); + if (i < statement.length() && statement.charAt(i) == '(') { + i = skipBlanks(statement, i + 1); + } + if (i < statement.length() + && (statement.charAt(i) == '\'' || statement.charAt(i) == '"')) { + char q = statement.charAt(i); + int end = statement.indexOf(q, i + 1); + if (end > i) { + return statement.substring(i + 1, end); + } + } + at = statement.indexOf(STRICTLY, at + 1); + } + return null; + } + + /** Numeric dotted version compare; a missing segment counts as zero. */ + private static int compareVersions(String left, String right) { + String[] l = left.split("\\."); + String[] r = right.split("\\."); + int len = Math.max(l.length, r.length); + for (int i = 0; i < len; i++) { + int a = i < l.length ? parseSegment(l[i]) : 0; + int b = i < r.length ? parseSegment(r[i]) : 0; + if (a != b) { + return a < b ? -1 : 1; + } + } + return 0; + } + + private static int parseSegment(String segment) { + try { + return Integer.parseInt(segment); + } catch (NumberFormatException notANumber) { + return 0; + } + } + /** * Whether the app actively declares this {@code org.jetbrains.kotlin} * artifact, rather than merely mentioning its name somewhere in a Gradle @@ -236,6 +344,16 @@ public static String constraintsBlock(String configuration, * *Two spellings count as a declaration -- the colon-joined coordinate * and the map form -- because those are what a pin is actually written as. + * A declaration inside {@code if (project.hasProperty('x'))} counts as + * present, deliberately: whether it is in force is decided by Gradle at + * evaluation time and cannot be read out of the text. Treating it as + * present honours the documented promise at the cost of leaving a + * duplicate the app already had; the alternative -- suppressing only on a + * strict version, which is the one declaration a constraint cannot coexist + * with -- removes that hazard along with everything else in this method, + * and is a documented behaviour change rather than a bug fix, so it is a + * decision for the project rather than something to slip in under a review + * thread. * Anything else falls through to "not declared", which is the safe * direction: emitting a constraint the app did not need only raises an * artifact to a shim, while skipping one it did need fails the build.
diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index ce4f3b2583b..5f786016d86 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -375,6 +375,47 @@ public void aMapEntryMayHaveSpaceAroundItsColon() { "naming a different Kotlin artifact does not pin jdk8"); } + /** + * A strict pin on kotlin-stdlib itself blocks both shims, not one. The + * shim at this floor depends on kotlin-stdlib at the same floor, so an app + * strictly holding the base library below it cannot resolve either + * constraint -- and the pre-merge family it is holding had no duplicate to + * begin with, so constraining there turns a working build into + * "Could not resolve ... {strictly 1.7.22}". + */ + @Test + public void aStrictPinOnTheBaseStdlibBlocksBothShims() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib:1.7.22') " + + "{ version { strictly '1.7.22' } }\n"); + check("".equals(out), + "a strict pre-merge base pin writes no constraints at all"); + + // At or above the floor there is no conflict, so the block still goes in: + // a shim requiring 1.8.0 is satisfied by a strict 1.9.22. + String modern = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib:1.9.22') " + + "{ version { strictly '1.9.22' } }\n"); + check(modern.contains("kotlin-stdlib-jdk8:1.8.0"), + "a strict modern base pin does not need the block suppressed"); + } + + /** + * kotlin-stdlib is a prefix of kotlin-stdlib-jdk8, so the base match has to + * be exact. A loose one would read every shim declaration as a pin on the + * base library and switch the whole block off. + */ + @Test + public void aStrictShimPinIsNotAPinOnTheBaseStdlib() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22') " + + "{ version { strictly '1.7.22' } }\n"); + check(out.contains("kotlin-stdlib-jdk7:1.8.0"), + "pinning the jdk8 shim leaves jdk7 constrained, not the whole block off"); + check(!out.contains("kotlin-stdlib-jdk8:1.8.0"), + "and jdk8 itself is left to the app"); + } + /** * The block absorbed for that check belongs to the declaration that opened * it and no further. A dependencies or android block must not swallow the From 4ac657cb79e8bde66f8e28286e55468d6823518e Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:35:00 +0300 Subject: [PATCH 16/94] A prerelease of the floor is below it, and a reason string is not a declaration Two findings, the second of them caused by the fix for an earlier one. 1.8.0-RC2 is a published Kotlin version whose numeric part compares equal to 1.8.0, so a strict pin on it read as "at the floor" and the block was written -- whereupon the shims request the FINAL 1.8.0, which cannot coexist with the strict prerelease, and a resolvable graph stops resolving. A qualifier at the floor now counts as below it. A qualifier anywhere else is still ignored, since rounding 1.9.22-RC up to 1.9.22 keeps it above the floor either way. The second is worth naming as such: two rounds ago a quoted configuration name was accepted so that dependencies.add("runtimeOnly", "...") would be recognised, and that widening let the word inside `because 'implementation workaround'` read as a main-variant declaration -- suppressing the constraint for a dependency that only affects debug. A quoted configuration name now counts in exactly one position, as the first argument of an add() call, and the unquoted form is matched only outside strings. Both spellings have a case, and the add() case was already there to catch this going wrong in the other direction. Four new cases, both behaviours checked by putting the old ones back. Co-Authored-By: Claude Opus 5 (1M context)A prerelease of the floor is below it. {@code 1.8.0-RC2} is a + * published Kotlin version, and its numeric part compares equal to + * {@code 1.8.0} -- so without this it read as "at the floor" and the block + * was written, whereupon the shims request the FINAL 1.8.0 and cannot + * coexist with the app's strict prerelease. A qualifier on any other + * version is ignored, because rounding {@code 1.9.22-RC} up to + * {@code 1.9.22} keeps it above the floor either way.
+ * + *Unreadable counts as below, because the failure it guards against + * cannot be worked around by the app while the duplicate class it risks + * instead can.
+ */ + private static boolean belowTheFloor(String version) { + if (version == null) { + return true; + } + int compared = compareVersions(version, MERGED_STDLIB_FLOOR); + if (compared != 0) { + return compared < 0; + } + return hasQualifier(version); + } + + /** Whether the version carries anything after its numeric segments. */ + private static boolean hasQualifier(String version) { + for (int i = 0; i < version.length(); i++) { + char c = version.charAt(i); + if (c != '.' && !Character.isDigit(c)) { + return true; + } + } + return false; + } + /** Numeric dotted version compare; a missing segment counts as zero. */ private static int compareVersions(String left, String right) { String[] l = left.split("\\."); @@ -565,29 +600,65 @@ private static boolean declaresOnTheConstrainedConfiguration(String configuratio /** Whether this line declares on {@code configuration}, as a whole token. */ private static boolean declaresOn(String configuration, String line) { - int at = line.indexOf(configuration); - while (at >= 0) { - boolean startsToken = at == 0 - || !Character.isLetterOrDigit(line.charAt(at - 1)); - int after = at + configuration.length(); - // A closing quote ends the token too, for the map-free - // dependencies.add("runtimeOnly", "group:artifact:version") spelling. - // Safe to accept: a quoted configuration name only decides anything on a - // statement that also carries the artifact coordinate, and a statement - // carrying both is a declaration however it is spelled. - boolean endsToken = after < line.length() - && (line.charAt(after) == ' ' || line.charAt(after) == '(' - || line.charAt(after) == '\t' - || line.charAt(after) == '"' - || line.charAt(after) == '\''); - if (startsToken && endsToken) { - return true; + char quote = 0; + int stringStart = -1; + for (int i = 0; i < line.length(); i++) { + char c = line.charAt(i); + if (quote != 0) { + if (c == '\\' && i + 1 < line.length()) { + i++; + } else if (c == quote) { + // A configuration name inside a string counts in one place only: + // as the first argument of dependencies.add("runtimeOnly", ".."). + // Accepting any quoted occurrence read the word in a reason -- + // because 'implementation workaround' -- as a main-variant + // declaration, which suppressed the constraint for a dependency + // that only affects debug. + if (line.substring(stringStart + 1, i).equals(configuration) + && isAddCallArgument(line, stringStart)) { + return true; + } + quote = 0; + } + continue; + } + if (c == '\'' || c == '"') { + quote = c; + stringStart = i; + continue; + } + if (line.startsWith(configuration, i)) { + boolean startsToken = i == 0 + || !Character.isLetterOrDigit(line.charAt(i - 1)); + int after = i + configuration.length(); + boolean endsToken = after < line.length() + && (line.charAt(after) == ' ' || line.charAt(after) == '(' + || line.charAt(after) == '\t'); + if (startsToken && endsToken) { + return true; + } } - at = line.indexOf(configuration, at + 1); } return false; } + /** Whether the string literal opening at {@code quoteAt} is an add() argument. */ + private static boolean isAddCallArgument(String line, int quoteAt) { + int i = quoteAt - 1; + while (i >= 0 && (line.charAt(i) == ' ' || line.charAt(i) == '\t')) { + i--; + } + if (i < 0 || line.charAt(i) != '(') { + return false; + } + i--; + while (i >= 0 && (line.charAt(i) == ' ' || line.charAt(i) == '\t')) { + i--; + } + return i >= 2 && "add".equals(line.substring(i - 2, i + 1)) + && (i - 3 < 0 || !Character.isLetterOrDigit(line.charAt(i - 3))); + } + /** * A fragment's lines with comments removed and exclusions dropped -- the * text that actually declares something. diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index 5f786016d86..93a24a7cad3 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -400,6 +400,49 @@ public void aStrictPinOnTheBaseStdlibBlocksBothShims() { "a strict modern base pin does not need the block suppressed"); } + /** + * A prerelease of the floor is below the floor. 1.8.0-RC2 is a published + * Kotlin version whose numeric part compares equal to 1.8.0, so it read as + * "at the floor" and the block was written -- whereupon the shims request + * the FINAL 1.8.0 and cannot coexist with the strict prerelease. + */ + @Test + public void aPrereleaseOfTheFloorCountsAsBelowIt() { + String rc = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib:1.8.0-RC2') " + + "{ version { strictly '1.8.0-RC2' } }\n"); + check("".equals(rc), "a strict prerelease of the floor suppresses the block"); + + // A qualifier above the floor changes nothing: rounding up keeps it above. + String later = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib:1.9.22-RC') " + + "{ version { strictly '1.9.22-RC' } }\n"); + check(later.contains("kotlin-stdlib-jdk8:1.8.0"), + "a prerelease above the floor still gets the constraints"); + } + + /** + * A configuration name inside a reason string is prose. Accepting any + * quoted occurrence -- which the dependencies.add spelling needed -- read + * `because 'implementation workaround'` as a main-variant declaration and + * suppressed the constraint for a dependency affecting only debug. + */ + @Test + public void aConfigurationNameInAReasonStringIsNotADeclaration() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " debugImplementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22') " + + "{ because 'implementation workaround' }\n"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), + "a reason mentioning the configuration does not make it a declaration"); + + // and the add() spelling it was widened for still works + String add = KotlinStdlibAlignment.constraintsBlock("implementation", + " dependencies.add(\"runtimeOnly\", " + + "\"org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22\")\n"); + check(!add.contains("kotlin-stdlib-jdk8:1.8.0"), + "the add() spelling is still recognised"); + } + /** * kotlin-stdlib is a prefix of kotlin-stdlib-jdk8, so the base match has to * be exact. A loose one would read every shim declaration as a pin on the From 98d576950c2914996f3c2e544471fb137225c522 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:44:54 +0300 Subject: [PATCH 17/94] Read the declaration Gradle reads: next-line closures, prose, qualified segments Three findings, all measured rather than reasoned about. A trailing closure may sit on the line AFTER the call's closing parenthesis. That looked like invalid Groovy, so it was checked rather than dismissed: Gradle accepts it, and the `strictly` inside really does apply -- confirmed by putting a competing higher requirement beside it and watching resolution fail with "Could not resolve ... {strictly 1.7.22}". The parenthesis depth is already back to zero at that newline, so the closure landed in its own statement and the version it carried was never associated with the coordinate above it. A statement that is nothing but the start of a closure now joins the one before it, and only for a statement that already names the Kotlin group. A coordinate inside a reason is prose. `because 'avoid org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'` was read as a declaration and dropped the constraint for an artifact nobody had pinned. "Outside a string" cannot be the test here the way it is for `strictly` or a configuration name, because a coordinate lives in a string -- what separates them is that a declaration's string OPENS with the coordinate while prose merely contains it. That also subsumes the exact-artifact rule the base-stdlib check needed, so both now go through one matcher. A qualified version segment keeps its number. Reading `20-RC` as zero made 1.8.20-RC compare equal to the 1.8.0 floor, and the qualifier rule then classified a version well ABOVE the floor as below it -- suppressing an alignment that was needed. The prerelease OF the floor is still below it, and both directions have a case. Six new cases; all three behaviours checked by putting the old ones back. Co-Authored-By: Claude Opus 5 (1M context)A coordinate lives inside a string, so "outside a string" cannot be + * the test the way it is for {@code strictly} or a configuration name. + * What separates the two is where in the string it sits: + * {@code 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'} opens with it, + * while {@code because 'avoid org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'} + * is prose that happens to contain it -- and reading that prose as a + * declaration dropped the constraint for an artifact nobody had pinned.
+ * + *The artifact is matched exactly: {@code kotlin-stdlib} is a prefix of + * {@code kotlin-stdlib-jdk8}, so what follows the name has to be the + * version separator or the end of the literal.
+ */ + private static boolean namesCoordinate(String line, String artifact) { + String coordinate = KOTLIN_GROUP + ":" + artifact; + char quote = 0; + int stringStart = -1; + for (int i = 0; i < line.length(); i++) { + char c = line.charAt(i); + if (quote != 0) { + if (c == '\\' && i + 1 < line.length()) { + i++; + } else if (c == quote) { + String literal = line.substring(stringStart + 1, i); + if (literal.equals(coordinate) + || literal.startsWith(coordinate + ":")) { + return true; + } + quote = 0; + } + continue; + } + if (c == '\'' || c == '"') { + quote = c; + stringStart = i; } - at = line.indexOf(coordinate, at + 1); } - return line.contains(KOTLIN_GROUP) && declaresMapEntry(line, "name", BASE_STDLIB); + return false; } /** The version inside this statement's {@code strictly} call, or null. */ @@ -350,10 +384,23 @@ private static int compareVersions(String left, String right) { return 0; } + /** + * A version segment's leading digits. {@code 20-RC} is 20, not zero: + * reading it as zero made {@code 1.8.20-RC} compare equal to the 1.8.0 + * floor, and the qualifier rule then classified a version well ABOVE the + * floor as below it, suppressing an alignment that was needed. + */ private static int parseSegment(String segment) { + int to = 0; + while (to < segment.length() && Character.isDigit(segment.charAt(to))) { + to++; + } + if (to == 0) { + return 0; + } try { - return Integer.parseInt(segment); - } catch (NumberFormatException notANumber) { + return Integer.parseInt(segment.substring(0, to)); + } catch (NumberFormatException tooLong) { return 0; } } @@ -442,7 +489,7 @@ private static boolean declaresArtifactOnLine(String artifact, String configurat && !declaresOnTheConstrainedConfiguration(configuration, line)) { return false; } - if (line.contains(KOTLIN_GROUP + ":" + artifact)) { + if (namesCoordinate(line, artifact)) { return true; } // group: 'org.jetbrains.kotlin', name: 'kotlin-stdlib-jdk8', version: '...' @@ -839,6 +886,16 @@ private static String[] statements(String text) { for (int i = 0; i < out.size(); i++) { String statement = out.get(i); if (statement.contains(KOTLIN_GROUP)) { + // A trailing closure may sit on the line AFTER the call's closing + // parenthesis -- Gradle accepts it and the strictly inside really does + // apply, checked by watching a competing higher requirement fail + // against it. The parenthesis depth is already back to zero there, so + // without this the closure lands in its own statement and the version + // it carries is never associated with the coordinate above it. + while (i + 1 < out.size() && opensAClosure(out.get(i + 1))) { + i++; + statement = statement + " " + out.get(i); + } int braces = braceBalance(statement); while (braces > 0 && i + 1 < out.size()) { i++; @@ -851,6 +908,12 @@ private static String[] statements(String text) { return merged.toArray(new String[merged.size()]); } + /** Whether the statement is nothing but the start of a closure. */ + private static boolean opensAClosure(String statement) { + String trimmed = statement.trim(); + return trimmed.startsWith("{"); + } + /** How far a statement opens or closes braces, ignoring those in strings. */ private static int braceBalance(String statement) { int depth = 0; diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index 93a24a7cad3..6e0f3d607f9 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -400,6 +400,68 @@ public void aStrictPinOnTheBaseStdlibBlocksBothShims() { "a strict modern base pin does not need the block suppressed"); } + /** + * A trailing closure may sit on the line after the call's closing + * parenthesis. Gradle accepts it and the {@code strictly} inside really + * does apply -- checked by watching a competing higher requirement fail + * against it -- but the parenthesis depth is back to zero there, so the + * closure landed in its own statement and its version was never + * associated with the coordinate above it. + */ + @Test + public void aTrailingClosureOnTheNextLineBelongsToTheDeclaration() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation(\n" + + " 'org.jetbrains.kotlin:kotlin-stdlib:1.7.22'\n" + + " )\n" + + " { version { strictly '1.7.22' } }\n"); + check("".equals(out), + "the strict pin in a next-line closure still suppresses the block"); + } + + /** + * A coordinate inside a reason is prose. A coordinate lives in a string, + * so "outside a string" cannot be the test here the way it is for + * strictly -- what separates them is that a declaration's string OPENS + * with the coordinate while prose merely contains it. + */ + @Test + public void aCoordinateInsideAReasonIsNotADeclaration() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('com.example:other:1.0') " + + "{ because 'avoid org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22' }\n"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), + "a coordinate mentioned in a reason does not count as a pin"); + + // the real notation still does + String real = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); + check(!real.contains("kotlin-stdlib-jdk8:1.8.0"), + "the dependency notation itself still counts"); + } + + /** + * A qualified segment keeps its number. Reading {@code 20-RC} as zero made + * 1.8.20-RC compare equal to the floor, and the qualifier rule then + * classified a version well ABOVE the floor as below it -- suppressing an + * alignment that was needed. + */ + @Test + public void aQualifiedSegmentKeepsItsNumber() { + String above = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib:1.8.20-RC') " + + "{ version { strictly '1.8.20-RC' } }\n"); + check(above.contains("kotlin-stdlib-jdk8:1.8.0"), + "a prerelease above the floor still gets the constraints"); + + // and the prerelease OF the floor is still below it + String atTheFloor = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib:1.8.0-RC2') " + + "{ version { strictly '1.8.0-RC2' } }\n"); + check("".equals(atTheFloor), + "a prerelease of the floor is still below it"); + } + /** * A prerelease of the floor is below the floor. 1.8.0-RC2 is a published * Kotlin version whose numeric part compares equal to 1.8.0, so it read as From f9ae6ff486ce22c1aacb256ac29990903ba60d4b Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:49:14 +0300 Subject: [PATCH 18/94] An underscore is part of an identifier A configuration named custom_implementation ended its embedded "implementation" on a boundary that looked clean, because isLetterOrDigit says an underscore is neither -- so it read as the main configuration and suppressed a constraint for a configuration that reaches nothing. The same held for implementation_extra and for every other token this file bounds: the check is shared by the configuration match, the strictly call, the map key and the add() call, and all four were wrong in the same way. Groovy identifier characters are now rejected on both sides of every token. Three cases: the prefixed form, the suffixed form, and the real configuration still counting, checked by putting isLetterOrDigit back. Co-Authored-By: Claude Opus 5 (1M context)Not {@code isLetterOrDigit}: an underscore is neither, so a + * configuration called {@code custom_implementation} ended its embedded + * {@code implementation} on a boundary that looked clean and was read as + * the main configuration -- suppressing a constraint for a configuration + * that reaches nothing.
+ */ + private static boolean isIdentifierChar(char c) { + return Character.isLetterOrDigit(c) || c == '_' || c == '$'; + } + private static int skipBlanks(String line, int from) { int i = from; while (i < line.length() && (line.charAt(i) == ' ' || line.charAt(i) == '\t')) { @@ -676,7 +689,7 @@ && isAddCallArgument(line, stringStart)) { } if (line.startsWith(configuration, i)) { boolean startsToken = i == 0 - || !Character.isLetterOrDigit(line.charAt(i - 1)); + || !isIdentifierChar(line.charAt(i - 1)); int after = i + configuration.length(); boolean endsToken = after < line.length() && (line.charAt(after) == ' ' || line.charAt(after) == '(' @@ -703,7 +716,7 @@ private static boolean isAddCallArgument(String line, int quoteAt) { i--; } return i >= 2 && "add".equals(line.substring(i - 2, i + 1)) - && (i - 3 < 0 || !Character.isLetterOrDigit(line.charAt(i - 3))); + && (i - 3 < 0 || !isIdentifierChar(line.charAt(i - 3))); } /** diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index 6e0f3d607f9..cd7eb32994a 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -400,6 +400,33 @@ public void aStrictPinOnTheBaseStdlibBlocksBothShims() { "a strict modern base pin does not need the block suppressed"); } + /** + * An underscore is an identifier character. A configuration named + * custom_implementation ended its embedded "implementation" on a boundary + * that looked clean, so it read as the main configuration and suppressed a + * constraint for a configuration that reaches nothing. + */ + @Test + public void aCustomConfigurationIsNotTheMainOne() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " custom_implementation " + + "'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'\n"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), + "custom_implementation is not the configuration being constrained"); + + String suffixed = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation_extra " + + "'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'\n"); + check(suffixed.contains("kotlin-stdlib-jdk8:1.8.0"), + "nor is implementation_extra"); + + // and the real one still is + String real = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'\n"); + check(!real.contains("kotlin-stdlib-jdk8:1.8.0"), + "the main configuration still counts"); + } + /** * A trailing closure may sit on the line after the call's closing * parenthesis. Gradle accepts it and the {@code strictly} inside really From a141d811b212e47b9307f78b6c37240dddf3671a Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 17:00:36 +0300 Subject: [PATCH 19/94] One rule for walking a string literal, since several had drifted apart Two findings, and they are the same finding twice: this class had grown several private copies of "walk to the closing quote", some honouring backslash escapes and some not, and every divergence became a defect. strictVersionIn found the strictly call correctly and then read the version with a plain search, so a reason like because "strictly '1.7.22' is not intended" supplied the number that decided whether the block was written braceBalance stopped at the apostrophe inside 'can\'t', ignored the declaration's real closing brace, and merged the following dependency into the Kotlin statement -- where an unrelated strict pin then read as one on kotlin-stdlib Both now call one endOfStringLiteral, as do callsStrictly and the brace counter, so a fix reaches all of them rather than the one that was reported. strictVersionIn also matches the token the way callsStrictly does instead of by substring, so the two cannot disagree about which call they are looking at. Two new cases, and the second of them was wrong on the first attempt: it asserted jdk8 stayed constrained, which stays true whether or not the statements merge, so it passed with the bug reinstated. It now names the base stdlib without a strict version beside an unrelated strict pin, where merging makes the whole block disappear -- and it fails when the escape handling is removed, which is the only evidence that a case is worth having. Co-Authored-By: Claude Opus 5 (1M context)One implementation because there were several, and they drifted. Each + * scanner in this class had its own copy of "walk to the closing quote", + * some honouring backslash escapes and some not, and every divergence + * turned into a defect: a statement scanner that stopped at the apostrophe + * inside {@code 'can\'t'} merged statements that must stay apart, and a + * brace counter that did the same swallowed a declaration's closing brace. + * They call this now, so a fix reaches all of them.
+ */ + private static int endOfStringLiteral(String text, int quoteAt) { + char quote = text.charAt(quoteAt); + for (int i = quoteAt + 1; i < text.length(); i++) { + char c = text.charAt(i); + if (c == '\\') { + i++; + } else if (c == quote) { + return i; + } + } + return text.length(); + } + private static int skipBlanks(String line, int from) { int i = from; while (i < line.length() && (line.charAt(i) == ' ' || line.charAt(i) == '\t')) { @@ -930,15 +962,10 @@ private static boolean opensAClosure(String statement) { /** How far a statement opens or closes braces, ignoring those in strings. */ private static int braceBalance(String statement) { int depth = 0; - char quote = 0; for (int i = 0; i < statement.length(); i++) { char c = statement.charAt(i); - if (quote != 0) { - if (c == quote) { - quote = 0; - } - } else if (c == '\'' || c == '"') { - quote = c; + if (c == '\'' || c == '"') { + i = endOfStringLiteral(statement, i); } else if (c == '{') { depth++; } else if (c == '}') { diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index cd7eb32994a..2bf575eee1a 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -400,6 +400,45 @@ public void aStrictPinOnTheBaseStdlibBlocksBothShims() { "a strict modern base pin does not need the block suppressed"); } + /** + * The version comes from the strict call, not from a reason that mentions + * one. Finding the call correctly and then reading the version with a + * plain search let prose supply it, so a declaration whose real strict + * version is compatible was judged on a number from its own comment. + */ + @Test + public void theStrictVersionComesFromTheCallNotTheProse() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib:1.9.22') " + + "{ because \"strictly '1.7.22' is not intended\"; " + + "version { strictly '1.9.22' } }\n"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), + "the real strict version is above the floor, so the block is written"); + } + + /** + * Brace balancing honours escapes, as the statement scanner does. A + * declaration whose closure contained an escaped apostrophe had its real + * closing brace ignored, so following dependencies merged into it and an + * unrelated strict pin could be read as one on kotlin-stdlib. + */ + @Test + public void anEscapedQuoteDoesNotSwallowAClosingBrace() { + // The base stdlib named without a strict version, then an UNRELATED strict + // pin. Correct: neither suppresses, so the block is written. With the escape + // mishandled the two statements merge, the merged statement both names + // kotlin-stdlib and calls strictly '1.7.22', and the whole block disappears. + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib:1.9.22') " + + "{ because 'can\\'t' }\n" + + " implementation('com.example:other:1.0') " + + "{ version { strictly '1.7.22' } }\n"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), + "an unrelated strict pin does not merge into the Kotlin declaration"); + check(out.contains("kotlin-stdlib-jdk7:1.8.0"), + "and the block is written in full"); + } + /** * An underscore is an identifier character. A configuration named * custom_implementation ended its embedded "implementation" on a boundary From 786d1bcd7fd83e09a36f34f2b5a2d1ad24dd6342 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 17:12:45 +0300 Subject: [PATCH 20/94] Scan the plugin fragment, join comma-continued maps, and stop at the block brace Three findings, one of them a gap in an earlier sweep of mine. android.gradlePlugin is scanned now. When android.supportv4Dep was found missing, the fix was to take the enumeration from ShieldInjector's GRADLE_TEXT_HINTS rather than from memory -- and then this one was classified from its name as landing in the plugin area. It does not: it is interpolated at top level right after `apply plugin`, where a dependencies block of its own is valid and reaches the same configurations. Reading the list was right; deciding what each entry does without looking at the interpolation site was not. Groovy's parenthesis-free map notation spreads one declaration over several lines held together by trailing commas. Splitting at those newlines left the configuration, the group, the artifact and any closure in four statements, none of which is a declaration on its own, so a strict pre-1.8 pin written that way was missed and the constraint made resolution fail. A trailing comma continues the statement now. An enclosing block's opening brace is not the declaration's own closure. A fragment putting its first dependency on the same line as `dependencies {` made that declaration swallow every following statement up to the closing brace, and an unrelated strict pin further down then read as a pin on the stdlib and silenced the whole block. The first attempt at that last one counted braces after the LAST string literal and broke an existing case: a trailing closure carries strings of its own -- an exclusion's module name, a strict version -- so counting after those missed the closure's own opening brace. It counts from the COORDINATE literal instead, which is the thing a block opener precedes and a trailing closure follows. The existing case is what caught it, which is the argument for keeping cases that look redundant. Three new cases, all three failing with their fixes reverted. Co-Authored-By: Claude Opus 5 (1M context)Counting the whole statement caught the ENCLOSING block's opener when + * a fragment put its first dependency on the same line as it -- + * {@code dependencies { implementation 'org.jetbrains.kotlin:kotlin-stdlib:1.9.22'} + * -- and the declaration then swallowed every following statement up to the + * closing brace, so an unrelated {@code strictly} further down read as a + * pin on the stdlib and silenced the whole block. A block opener sits + * BEFORE the coordinate and a trailing closure after it, so counting from + * the end of the last string literal separates them.
+ */ + private static int trailingBraceBalance(String statement) { + // From the end of the COORDINATE literal, not the last literal: a trailing + // closure carries strings of its own -- an exclusion's module name, a strict + // version -- and counting after those missed the closure's own opening brace. + for (int i = 0; i < statement.length(); i++) { + char c = statement.charAt(i); + if (c != '\'' && c != '"') { + continue; + } + int end = endOfStringLiteral(statement, i); + if (statement.substring(i + 1, Math.min(end, statement.length())) + .startsWith(KOTLIN_GROUP)) { + return braceBalance(statement.substring(Math.min(end + 1, + statement.length()))); + } + i = end; + } + return braceBalance(statement); + } + /** How far a statement opens or closes braces, ignoring those in strings. */ private static int braceBalance(String statement) { int depth = 0; diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index 2bf575eee1a..750c4ace9d9 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -400,6 +400,63 @@ public void aStrictPinOnTheBaseStdlibBlocksBothShims() { "a strict modern base pin does not need the block suppressed"); } + /** + * Groovy's parenthesis-free map notation spreads one declaration over + * several lines, held together by trailing commas. Splitting at those + * newlines left the configuration, the group, the artifact and the closure + * in four statements, none of which is a declaration on its own -- so a + * strict pre-1.8 pin written that way was missed and the constraint made + * resolution fail. + */ + @Test + public void aCommaContinuesAMultilineMapDeclaration() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation group: 'org.jetbrains.kotlin',\n" + + " name: 'kotlin-stdlib-jdk8',\n" + + " version: '1.7.22'\n"); + check(!out.contains("kotlin-stdlib-jdk8:1.8.0"), + "a comma-continued map declaration pins jdk8"); + check(out.contains("kotlin-stdlib-jdk7:1.8.0"), + "and leaves jdk7 constrained"); + } + + /** + * An enclosing block's opening brace is not the declaration's own closure. + * A fragment putting its first dependency on the same line as + * {@code dependencies {} made that declaration swallow every following + * statement up to the closing brace, so an unrelated strict pin further + * down read as one on the stdlib and silenced the whole block. + */ + @Test + public void anEnclosingBlockBraceIsNotATrailingClosure() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + "dependencies { implementation 'org.jetbrains.kotlin:kotlin-stdlib:1.9.22'\n" + + " implementation('com.example:other:1.0') " + + "{ version { strictly '1.7.22' } }\n" + + "}\n"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), + "the unrelated strict pin does not attach to the stdlib declaration"); + } + + /** + * android.gradlePlugin is interpolated at top level right after + * `apply plugin`, where a dependencies block of its own is valid and + * reaches the same configurations -- so it has to be scanned like the + * other app-controlled fragments. It was missed the same way + * android.supportv4Dep was. + */ + @Test + public void theBuilderScansTheGradlePluginFragment() throws Exception { + byte[] bytes = java.nio.file.Files.readAllBytes(new java.io.File( + "src/main/java/com/codename1/builders/AndroidGradleBuilder.java").toPath()); + String builderSrc = new String(bytes, "UTF-8"); + int at = builderSrc.indexOf("KotlinStdlibAlignment.constraintsBlock("); + check(at >= 0, "the builder calls the alignment"); + String call = builderSrc.substring(at, builderSrc.indexOf(";", at)); + check(call.contains("request.getArg(\"android.gradlePlugin\", \"\")"), + "android.gradlePlugin reaches the generated script and must be scanned"); + } + /** * The version comes from the strict call, not from a reason that mentions * one. Finding the call correctly and then reading the version with a From aea0eaeaaab4c60567a7e126b03ac07db714e274 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 17:25:56 +0300 Subject: [PATCH 21/94] Follow one hop through a def, and stop calling prose a coordinate Two findings, and a boundary drawn on purpose. A coordinate can sit one hop away behind a variable: def jdk8 = 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22' implementation(jdk8) { version { strictly '1.7.22' } } Neither statement carries both the configuration and the coordinate, so the strict pin was invisible and the constraint made the build stop resolving. A `def name = 'literal'` is folded into the statements that use the name. That is where following the value stops, and the limit is written down rather than left to be discovered. A coordinate assembled by concatenation, or built from a map, a list or a method call, is not in the text as a coordinate at all; recovering it needs Gradle to evaluate the script, which nothing here can do. There is a case pinning that the concatenated form is left unrecognised, so the next reader sees a decision rather than an oversight. An interpolated VERSION is a different matter and is still recognised -- the artifact name is literal there, and naming the artifact is what matters. Writing that case is what showed the first attempt at it had asserted the wrong thing. Separately: a reason that OPENS with the coordinate is still a reason. `because 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22 causes duplicate classes'` was read as the declaration it warns about, switching off the constraint that prevents exactly what it describes. Dependency notation carries no whitespace; prose does. Four new cases. Both fixes fail with their old behaviour restored. Co-Authored-By: Claude Opus 5 (1M context)A coordinate can sit one hop away:
+ * + *
+ * def jdk8 = 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'
+ * implementation(jdk8) { version { strictly '1.7.22' } }
+ *
+ *
+ * Neither statement carries both the configuration and the coordinate, + * so the strict pin was invisible and the constraint made the build stop + * resolving. One hop through a string literal is recoverable from the text + * and is recovered here.
+ * + *Where this stops, deliberately. A value built by + * interpolation, by concatenation, from a map or a list, or returned by a + * method is not in the text at all -- reading it needs Gradle to evaluate + * the script, which nothing here can do. Those forms are left unrecognised + * rather than guessed at, and that is a real limit of reading declarations + * out of build-hint text rather than something another pass would fix. The + * design that does not need to find the declaration at all -- constraining + * unless a strict version says otherwise -- is the answer to that class, + * and it is a decision about documented behaviour rather than a defect to + * patch here.
+ */ + private static String[] inlineLiteralDefinitions(ListA version this cannot read counts as below it. An app naming one of + * these artifacts at all is managing the family, and the harm of assuming + * the worst is an alignment not written for an app that had already sorted + * itself out, against a duplicate class manufactured in one that had.
+ */ + private static boolean declaredBelowTheFloor(String artifact, + String[] appGradleFragments) { + if (appGradleFragments == null) { + return false; + } + for (int i = 0; i < appGradleFragments.length; i++) { + String[] lines = activeLines(appGradleFragments[i]); + for (int j = 0; j < lines.length; j++) { + if (!declaresArtifactOnLine(artifact, "implementation", lines[j]) + && !declaresArtifactOnLine(artifact, "api", lines[j])) { + continue; + } + if (!namesArtifactAnywhere(lines[j], artifact)) { + continue; + } + if (belowTheFloor(declaredVersionOf(lines[j], artifact))) { + return true; + } + } + } + return false; + } + + /** Whether the statement names the artifact, in either spelling. */ + private static boolean namesArtifactAnywhere(String line, String artifact) { + return namesCoordinate(line, artifact) + || (line.contains(KOTLIN_GROUP) && declaresMapEntry(line, "name", artifact)); + } + + /** + * The version this statement declares for {@code artifact}: the third + * segment of its coordinate, or the map form's {@code version:} entry. + * Null when neither is readable, which callers treat as below the floor. + */ + private static String declaredVersionOf(String line, String artifact) { + String coordinate = KOTLIN_GROUP + ":" + artifact + ":"; + for (int i = 0; i < line.length(); i++) { + char c = line.charAt(i); + if (c != '\'' && c != '"') { + continue; + } + int end = endOfStringLiteral(line, i); + String literal = line.substring(i + 1, Math.min(end, line.length())); + if (literal.startsWith(coordinate) && !hasWhitespace(literal)) { + return literal.substring(coordinate.length()); + } + i = end; + } + return mapEntryValue(line, "version"); + } + + /** The value of a {@code key: 'value'} map entry, or null. */ + private static String mapEntryValue(String line, String key) { + int at = line.indexOf(key); + while (at >= 0) { + boolean startsToken = at == 0 || !isIdentifierChar(line.charAt(at - 1)); + if (startsToken) { + int i = skipBlanks(line, at + key.length()); + if (i < line.length() && line.charAt(i) == ':') { + i = skipBlanks(line, i + 1); + if (i < line.length() + && (line.charAt(i) == '\'' || line.charAt(i) == '"')) { + int end = endOfStringLiteral(line, i); + if (end < line.length()) { + return line.substring(i + 1, end); + } + } + } + } + at = line.indexOf(key, at + 1); + } + return null; + } + /** * Whether the app strictly holds {@code kotlin-stdlib} itself below the * floor both shims depend on. @@ -953,14 +1046,21 @@ private static String[] statements(String text) { out.add(current.toString().replace('\n', ' ')); } } + // Definitions are folded in FIRST, because the merge below only absorbs a + // closure into a statement that already names the Kotlin group -- and a + // statement referring to the coordinate through a variable does not name it + // until the fold has happened. Merging first left `implementation(stdlib) {` + // unmerged, so its `strictly` was never associated with the coordinate. + ListThe KEY is looked for outside string literals only. A reason quoting + * the map form -- {@code because "avoid group: 'org.jetbrains.kotlin', + * name: 'kotlin-stdlib-jdk8'"} -- otherwise read as a declaration of that + * artifact, and since prose carries no version the whole block was + * suppressed. Same rule as the coordinate matcher beside it, which is + * where this had drifted apart from.
+ */ private static String mapEntryValue(String line, String key) { - int at = line.indexOf(key); - while (at >= 0) { - boolean startsToken = at == 0 || !isIdentifierChar(line.charAt(at - 1)); - if (startsToken) { - int i = skipBlanks(line, at + key.length()); - if (i < line.length() && line.charAt(i) == ':') { - i = skipBlanks(line, i + 1); - if (i < line.length() - && (line.charAt(i) == '\'' || line.charAt(i) == '"')) { - int end = endOfStringLiteral(line, i); - if (end < line.length()) { - return line.substring(i + 1, end); - } - } + for (int i = 0; i < line.length(); i++) { + char c = line.charAt(i); + if (c == '\'' || c == '"') { + i = endOfStringLiteral(line, i); + continue; + } + if (!line.startsWith(key, i)) { + continue; + } + boolean startsToken = i == 0 || !isIdentifierChar(line.charAt(i - 1)); + int after = i + key.length(); + if (!startsToken || (after < line.length() + && isIdentifierChar(line.charAt(after)))) { + continue; + } + int j = skipBlanks(line, after); + if (j >= line.length() || line.charAt(j) != ':') { + continue; + } + j = skipBlanks(line, j + 1); + if (j < line.length() && (line.charAt(j) == '\'' || line.charAt(j) == '"')) { + int end = endOfStringLiteral(line, j); + if (end < line.length()) { + return line.substring(j + 1, end); } } - at = line.indexOf(key, at + 1); + i = j; } return null; } @@ -345,10 +369,11 @@ private static boolean strictlyPinsBaseStdlibBelowTheFloor(String[] appGradleFra for (int i = 0; i < appGradleFragments.length; i++) { String[] lines = activeLines(appGradleFragments[i]); for (int j = 0; j < lines.length; j++) { - if (!callsStrictly(lines[j]) || !namesBaseStdlib(lines[j])) { + if (!namesBaseStdlib(lines[j])) { continue; } - if (belowTheFloor(strictVersionIn(lines[j]))) { + String strict = strictVersionOfBaseStdlib(lines[j]); + if (strict != null && belowTheFloor(strict)) { return true; } } @@ -356,6 +381,35 @@ private static boolean strictlyPinsBaseStdlibBelowTheFloor(String[] appGradleFra return false; } + /** + * The version this statement strictly holds {@code kotlin-stdlib} at, or + * null when it does not hold it strictly. + * + *Two spellings mean the same thing. The {@code strictly} call is one; + * Gradle's {@code !!} suffix on the version is the other, and missing it + * was not a near miss. Measured with Gradle: an app writing + * {@code 'org.jetbrains.kotlin:kotlin-stdlib:1.7.22!!'} beside a + * pre-merge jdk8 resolves the coherent 1.7.22 family on its own, and with + * this block's constraints added resolves kotlin-stdlib 1.7.22 beside + * jdk7/jdk8 1.8.0 -- the EMPTY shims. The jdk extension classes are then + * supplied by neither jar, and the app fails at runtime with a missing + * class rather than at build time with a duplicate one. That is the worst + * outcome available here, so the suffix is read as what it is.
+ */ + private static String strictVersionOfBaseStdlib(String line) { + if (callsStrictly(line)) { + return strictVersionIn(line); + } + String declared = declaredVersionOf(line, BASE_STDLIB); + if (declared != null && declared.endsWith(STRICT_SUFFIX)) { + return declared.substring(0, declared.length() - STRICT_SUFFIX.length()); + } + return null; + } + + /** Gradle's strict-version shorthand, written after the version. */ + private static final String STRICT_SUFFIX = "!!"; + /** Whether the statement names {@code kotlin-stdlib} and not a longer name. */ private static boolean namesBaseStdlib(String line) { return namesCoordinate(line, BASE_STDLIB) @@ -671,27 +725,8 @@ private static boolean callsStrictly(String statement) { * failed resolution. */ private static boolean declaresMapEntry(String line, String key, String value) { - int at = line.indexOf(key); - while (at >= 0) { - boolean startsToken = at == 0 - || !isIdentifierChar(line.charAt(at - 1)); - if (startsToken) { - int i = skipBlanks(line, at + key.length()); - if (i < line.length() && line.charAt(i) == ':') { - i = skipBlanks(line, i + 1); - if (i < line.length() - && (line.charAt(i) == '\'' || line.charAt(i) == '"')) { - char q = line.charAt(i); - int end = line.indexOf(q, i + 1); - if (end > i && line.substring(i + 1, end).equals(value)) { - return true; - } - } - } - } - at = line.indexOf(key, at + 1); - } - return false; + String found = mapEntryValue(line, key); + return found != null && found.equals(value); } /** diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index c967297e2bd..147396737ba 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -457,6 +457,59 @@ public void aDefReferenceWithAMultilineClosureIsStillAPin() { "the strict base pin behind a def with a multiline closure is honoured"); } + /** + * Gradle's {@code !!} suffix is the strict-version shorthand, and missing + * it produced the worst outcome available here. Measured: an app writing + * kotlin-stdlib:1.7.22!! beside a pre-merge jdk8 resolves the coherent + * 1.7.22 family on its own; with these constraints added it resolves + * kotlin-stdlib 1.7.22 beside jdk7/jdk8 1.8.0, the EMPTY shims -- so the + * jdk extension classes come from neither jar and the app fails at runtime + * with a missing class instead of at build time with a duplicate one. + */ + @Test + public void theStrictShorthandCountsAsAStrictPin() { + String base = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation 'org.jetbrains.kotlin:kotlin-stdlib:1.7.22!!'\n"); + check("".equals(base), + "a !! pin on the base stdlib suppresses both shims"); + + String shim = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22!!'\n"); + check("".equals(shim), "and a !! pin on a shim does too"); + + // Above the floor the shorthand changes nothing: the constraints are still + // satisfiable, so they are still written. + String modern = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation 'org.jetbrains.kotlin:kotlin-stdlib:1.9.22!!'\n"); + check(modern.contains("kotlin-stdlib-jdk8:1.8.0"), + "a merged-era !! pin does not need the block suppressed"); + } + + /** + * Map notation quoted inside a reason is prose too. The coordinate matcher + * had been taught to skip string literals and the map matcher beside it + * had not, so a reason naming the artifact in map form read as a + * declaration -- and since prose carries no version, the whole block was + * suppressed rather than one artifact. + */ + @Test + public void mapNotationInsideAReasonIsStillProse() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('com.example:other:1.0') { because " + + "\"avoid group: 'org.jetbrains.kotlin', name: 'kotlin-stdlib-jdk8'\" }\n"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), + "quoted map notation does not suppress"); + check(out.contains("kotlin-stdlib-jdk7:1.8.0"), + "and does not take the whole block with it"); + + // the real map form still counts + String real = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation group: 'org.jetbrains.kotlin', " + + "name: 'kotlin-stdlib-jdk8', version: '1.9.22'\n"); + check(!real.contains("kotlin-stdlib-jdk8:1.8.0"), + "a real map declaration still pins jdk8"); + } + /** * A reason that OPENS with the coordinate is still a reason. Accepting any * literal starting with one let a warning about the duplicate From 523e93fbfa97ae0eb7059823022c4df108669e8a Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 18:09:29 +0300 Subject: [PATCH 24/94] Take the version from a rich-version closure, not just from the coordinate A merged-era declaration can carry its version in the closure rather than in the coordinate: implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk7') { version { strictly '1.9.22' } } Reading no version there classified it as below the merge floor, and since a below-floor shim declaration suppresses BOTH constraints, it took the jdk8 constraint down with it -- the one that graph still needed. The strict version in the same declaration is used when the coordinate carries none. Two cases: the merged-era form leaves the sibling constraint standing, and the pre-merge form still takes both, which is what that rule exists for. Co-Authored-By: Claude Opus 5 (1M context)Both spellings count. Groovy's command syntax drops the parentheses -- + * {@code add 'implementation', 'group:artifact:version'} is as valid as + * {@code add("implementation", "...")} -- and requiring the parenthesis + * rejected a declaration that was carrying an explicit strict pin.
+ */ private static boolean isAddCallArgument(String line, int quoteAt) { int i = quoteAt - 1; while (i >= 0 && (line.charAt(i) == ' ' || line.charAt(i) == '\t')) { i--; } - if (i < 0 || line.charAt(i) != '(') { - return false; - } - i--; - while (i >= 0 && (line.charAt(i) == ' ' || line.charAt(i) == '\t')) { + if (i >= 0 && line.charAt(i) == '(') { i--; + while (i >= 0 && (line.charAt(i) == ' ' || line.charAt(i) == '\t')) { + i--; + } } return i >= 2 && "add".equals(line.substring(i - 2, i + 1)) && (i - 3 < 0 || !isIdentifierChar(line.charAt(i - 3))); diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index 712a7fec5da..dc903380627 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -457,6 +457,42 @@ public void aDefReferenceWithAMultilineClosureIsStillAPin() { "the strict base pin behind a def with a multiline closure is honoured"); } + /** + * Whether a declaration is strict and what version it is strict AT are two + * questions. Asking only the second let `version { strictly kotlinVersion }` + * read as not strict at all -- the opposite of the conservative path + * documented everywhere else, and the one case where being wrong costs a + * failed resolution rather than an override. + */ + @Test + public void aStrictPinWithAnUnreadableVersionStillSuppresses() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib:1.7.22') " + + "{ version { strictly kotlinVersion } }\n"); + check("".equals(out), + "a strict pin whose version cannot be read takes the conservative path"); + } + + /** + * Groovy's command syntax drops the parentheses, and requiring them + * rejected a declaration carrying an explicit strict pin. + */ + @Test + public void theParenthesisFreeAddFormCounts() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " add 'implementation', " + + "'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22!!'\n"); + check("".equals(out), + "add without parentheses is still an add"); + + // and a quoted configuration name with no add in front still counts for nothing + String bare = KotlinStdlibAlignment.constraintsBlock("implementation", + " def cfg = 'implementation'\n" + + " something cfg, 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'\n"); + check(bare.contains("kotlin-stdlib-jdk8:1.8.0"), + "a quoted configuration name without an add does not declare anything"); + } + /** * A rich-version closure carries the version instead of the coordinate. * Reading no version there made a merged-era declaration look below the From 858c49aa3f8c74f526c3cdc0096bd5e5ad762f75 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 18:24:27 +0300 Subject: [PATCH 26/94] Read a version selector by its lower bound, the declared group, and any rich version Three findings, all the same underlying mistake in three places: reading part of a declaration and treating "I could not parse that" as "the app pinned something old", which drops BOTH constraints and leaves the duplicate the sibling would have prevented. Gradle accepts more than a literal version, and every other shape parsed as zero. What matters is the lowest version a selector can resolve to: [1.8.0] exact range at the floor not below it 1.8.+ cannot resolve below 1.8.0 not below it [1.7.0,1.9.0) low end is pre-merge below it 1.7.+ below it 1.8.0-RC2 a prerelease OF the floor below it A prerelease and a dynamic marker are no longer the same thing: the first sorts below its own version, the second cannot go under its numeric prefix. The map form now matches the declared GROUP rather than the group appearing anywhere in the statement. A fork under another group whose reason mentioned org.jetbrains.kotlin combined with an unrelated artifact name and read as a Kotlin shim, and since its version was below the floor both constraints went. And a rich-version closure can name the version with a keyword other than strictly. Reading only strictly left `version { require '1.9.22' }` with no version at all, so a declaration that was already merged-era took its sibling's constraint down with it. strictly still decides STRICTNESS -- that has not changed -- but require and prefer answer "what version". Eight new cases across the three, each fix failing with its old behaviour restored, and four of the eight pinning the directions these could have overshot: the real map group still counts, a pre-merge required version still suppresses both, and a range reaching below the floor is still treated as below it. Co-Authored-By: Claude Opus 5 (1M context){@code strictly} is the one that changes whether the constraints can + * coexist with the app's, but it is not the only one that says what version + * is meant. Reading only it left {@code version { require '1.9.22' } } + * with no version at all, which the conservative path then treated as + * below the floor -- dropping BOTH constraints for a declaration that was + * already merged-era and needed only its sibling left alone.
+ */ + private static String richVersionIn(String statement) { + String strict = versionInCall(statement, STRICTLY); + if (strict != null) { + return strict; + } + String required = versionInCall(statement, "require"); + if (required != null) { + return required; + } + return versionInCall(statement, "prefer"); + } + + /** The quoted argument of {@code call}, found outside string literals. */ + private static String versionInCall(String statement, String call) { // The same syntax-level call callsStrictly validated, not any occurrence of // the word: a reason reading `because "strictly '1.7.22' is not intended"` // otherwise supplies the version for a declaration whose real strict version @@ -516,14 +546,14 @@ private static String strictVersionIn(String statement) { i = endOfStringLiteral(statement, i); continue; } - if (!statement.startsWith(STRICTLY, i)) { + if (!statement.startsWith(call, i)) { continue; } boolean startsToken = i == 0 || !isIdentifierChar(statement.charAt(i - 1)); if (!startsToken) { continue; } - int after = skipBlanks(statement, i + STRICTLY.length()); + int after = skipBlanks(statement, i + call.length()); if (after < statement.length() && statement.charAt(after) == '(') { after = skipBlanks(statement, after + 1); } @@ -559,20 +589,61 @@ private static boolean belowTheFloor(String version) { if (version == null) { return true; } - int compared = compareVersions(version, MERGED_STDLIB_FLOOR); + String lowerBound = lowerBoundOf(version); + if (lowerBound == null) { + return true; + } + int compared = compareVersions(lowerBound, MERGED_STDLIB_FLOOR); if (compared != 0) { return compared < 0; } - return hasQualifier(version); + // At the floor numerically, only a PRERELEASE is below it. A dynamic marker + // is not: 1.8.+ cannot resolve lower than 1.8.0, so it is at the floor and + // the constraints are still satisfiable. + return isPrerelease(lowerBound); } - /** Whether the version carries anything after its numeric segments. */ - private static boolean hasQualifier(String version) { + /** + * The lowest version a selector can resolve to, as far as the text says. + * + *Gradle accepts more than a literal here, and each shape was read as + * zero before: {@code [1.8.0]} is an exact range whose bracket stopped the + * numeric parse, {@code [1.7.0,1.9.0)} is a range whose LOW end is what + * matters for this question, and {@code 1.8.+} is a dynamic selector that + * cannot go below 1.8.0. Reading any of them as zero classified a + * merged-era declaration as pre-merge and dropped both constraints, + * including the sibling's -- which is the one such a graph still needs.
+ */ + private static String lowerBoundOf(String version) { + String selector = version.trim(); + if (selector.length() == 0) { + return null; + } + char opening = selector.charAt(0); + if (opening == '[' || opening == '(') { + selector = selector.substring(1); + int to = 0; + while (to < selector.length() && ",])".indexOf(selector.charAt(to)) < 0) { + to++; + } + selector = selector.substring(0, to); + } + selector = selector.trim(); + return selector.length() == 0 ? null : selector; + } + + /** + * Whether this version is a prerelease of its own numeric version, as + * opposed to a dynamic selector. {@code 1.8.0-RC2} sorts below + * {@code 1.8.0}; {@code 1.8.+} does not. + */ + private static boolean isPrerelease(String version) { for (int i = 0; i < version.length(); i++) { char c = version.charAt(i); - if (c != '.' && !Character.isDigit(c)) { - return true; + if (c == '.' || Character.isDigit(c)) { + continue; } + return c != '+'; } return false; } @@ -701,7 +772,8 @@ private static boolean declaresArtifactOnLine(String artifact, String configurat return true; } // group: 'org.jetbrains.kotlin', name: 'kotlin-stdlib-jdk8', version: '...' - return line.contains(KOTLIN_GROUP) && declaresMapEntry(line, "name", artifact); + return declaresMapEntry(line, "group", KOTLIN_GROUP) + && declaresMapEntry(line, "name", artifact); } /** diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index dc903380627..d448e1a7514 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -457,6 +457,91 @@ public void aDefReferenceWithAMultilineClosureIsStillAPin() { "the strict base pin behind a def with a multiline closure is honoured"); } + /** + * The map form has to match the declared GROUP, not the group appearing + * anywhere. A fork under another group whose reason merely mentions + * org.jetbrains.kotlin combined with an unrelated artifact name and read + * as a Kotlin shim -- and since its version was below the floor, both + * constraints went. + */ + @Test + public void theMapFormMatchesTheDeclaredGroup() { + String fork = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation(group: 'com.example', name: 'kotlin-stdlib-jdk8', " + + "version: '1.0') { because 'fork of org.jetbrains.kotlin' }\n"); + check(fork.contains("kotlin-stdlib-jdk8:1.8.0"), + "another group's artifact is not our shim"); + check(fork.contains("kotlin-stdlib-jdk7:1.8.0"), + "and it does not take the block with it"); + + // the real map form still counts + String real = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation group: 'org.jetbrains.kotlin', " + + "name: 'kotlin-stdlib-jdk8', version: '1.9.22'\n"); + check(!real.contains("kotlin-stdlib-jdk8:1.8.0"), + "the real group still pins jdk8"); + } + + /** + * A rich-version closure can say what version is meant with a keyword + * other than strictly. Reading only strictly left `version { require }` + * with no version, which the conservative path treated as below the floor + * -- dropping both constraints for a declaration already merged-era. + */ + @Test + public void aRequiredRichVersionIsAVersionToo() { + String required = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk7') " + + "{ version { require '1.9.22' } }\n"); + check(required.contains("kotlin-stdlib-jdk8:1.8.0"), + "a required merged-era version leaves the sibling constrained"); + + String preferred = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk7') " + + "{ version { prefer '1.9.22' } }\n"); + check(preferred.contains("kotlin-stdlib-jdk8:1.8.0"), + "and so does a preferred one"); + + // below the floor it still takes both + String old = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk7') " + + "{ version { require '1.7.22' } }\n"); + check("".equals(old), "a required pre-merge version still suppresses both"); + } + + /** + * Gradle accepts more than a literal version, and each shape parsed as + * zero before -- classifying a merged-era declaration as pre-merge and + * dropping BOTH constraints, including the sibling's, which is the one + * such a graph still needs. What matters is the lowest version the + * selector can resolve to. + */ + @Test + public void aVersionSelectorIsReadByItsLowerBound() { + // exact range at the floor: not below it + String exact = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk7:[1.8.0]'\n"); + check(exact.contains("kotlin-stdlib-jdk8:1.8.0"), + "an exact merged-era range leaves the sibling constrained"); + + // dynamic selector that cannot go below the floor + String dynamic = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.8.+'\n"); + check(dynamic.contains("kotlin-stdlib-jdk8:1.8.0"), + "1.8.+ cannot resolve below the floor"); + + // range whose low end IS below the floor: conservative, both go + String spanning = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk7:[1.7.0,1.9.0)'\n"); + check("".equals(spanning), + "a range reaching below the floor is treated as below it"); + + // and a dynamic selector below the floor likewise + String oldDynamic = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.7.+'\n"); + check("".equals(oldDynamic), "1.7.+ is below the floor"); + } + /** * Whether a declaration is strict and what version it is strict AT are two * questions. Asking only the second let `version { strictly kotlinVersion }` From 5270ddd5db5550a1426c7551d55b22c87ddc3799 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 18:37:04 +0300 Subject: [PATCH 27/94] Follow definitions in statement order, and through interpolation Two findings, both within the one-hop-through-a-literal rule this already follows rather than past the boundary it declines to cross. A known definition referred to as $name or ${name} inside a double-quoted string is the same hop as a bare token reference. Reading it as unreadable made a merged-era version look pre-merge, which -- since a below-floor shim declaration suppresses BOTH constraints -- dropped the sibling's along with it. An UNKNOWN name is still unreadable and still takes the conservative path, and concatenation is still not recovered: the boundary has not moved, only the inside of it is now consistent. Definitions are applied in statement order, and a definition is recorded after its own statement has been rewritten. The previous two-pass version built one map for the whole fragment, so a variable's value leaked backwards across a reassignment and a main-variant declaration standing ABOVE the reassignment read as a pin on a value it never held. A reassignment to something unreadable now forgets the name rather than leaving a stale literal in place, which has its own case. Five new cases. Both fixes fail with their old behaviour restored -- but the ordering one only after its case was rewritten. As first written it reassigned Kotlin-to-unrelated, which a last-wins map answers correctly by accident; it now reassigns unrelated-to-Kotlin, where only walking in order gets the earlier statement right. That is the fourth case this review that passed while proving nothing, and the regression check is the only reason any of them were found. Co-Authored-By: Claude Opus 5 (1M context)One predicate because there were two, and they diverged: the bypass + * that lets a strict pin escape the configuration filter asked only about + * the {@code strictly} keyword, so + * {@code debugImplementation '...jdk8:1.7.22!!'} was filtered out as a + * variant declaration and got the constraint anyway -- against a strict + * requirement that had resolved fine before it.
+ */ + private static boolean holdsStrictly(String line, String artifact) { if (callsStrictly(line)) { return true; } - String declared = declaredVersionOf(line, BASE_STDLIB); + String declared = declaredVersionOf(line, artifact); return declared != null && declared.endsWith(STRICT_SUFFIX); } @@ -764,7 +787,7 @@ private static boolean declaresArtifactOnLine(String artifact, String configurat // fails the whole script at evaluation, which is a far larger blast radius // than the case it fixes. Revisit only with a project that actually has this // shape. - if (!callsStrictly(line) + if (!holdsStrictly(line, artifact) && !declaresOnTheConstrainedConfiguration(configuration, line)) { return false; } @@ -854,6 +877,26 @@ private static boolean isIdentifierChar(char c) { */ private static int endOfStringLiteral(String text, int quoteAt) { char quote = text.charAt(quoteAt); + // Groovy's triple-quoted literals are a different delimiter, not three of + // this one. Treating the opener as a single quote made a triple-quoted note + // close on the first apostrophe it contains -- can't, in the case that found + // this -- and threw the rest of the fragment out of step, so a strict pin + // after it was never seen. + boolean tripled = quoteAt + 2 < text.length() + && text.charAt(quoteAt + 1) == quote + && text.charAt(quoteAt + 2) == quote; + if (tripled) { + for (int i = quoteAt + 3; i + 2 < text.length(); i++) { + char c = text.charAt(i); + if (c == '\\') { + i++; + } else if (c == quote && text.charAt(i + 1) == quote + && text.charAt(i + 2) == quote) { + return i + 2; + } + } + return text.length(); + } for (int i = quoteAt + 1; i < text.length(); i++) { char c = text.charAt(i); if (c == '\\') { @@ -1027,7 +1070,6 @@ private static String[] activeLines(String fragment) { } StringBuilder out = new StringBuilder(); boolean inBlockComment = false; - char quote = 0; for (int i = 0; i < fragment.length(); i++) { char c = fragment.charAt(i); if (inBlockComment) { @@ -1039,19 +1081,14 @@ private static String[] activeLines(String fragment) { } continue; } - if (quote != 0) { - out.append(c); - if (c == '\\' && i + 1 < fragment.length()) { - out.append(fragment.charAt(i + 1)); - i++; - } else if (c == quote) { - quote = 0; - } - continue; - } if (c == '\'' || c == '"') { - quote = c; - out.append(c); + // The shared rule, so triple-quoted literals and escapes are the + // same thing here as everywhere else. This scanner and the statement + // scanner below kept their own copies through the consolidation, and + // the triple-quote fix reached neither until now. + int end = endOfStringLiteral(fragment, i); + out.append(fragment, i, Math.min(end + 1, fragment.length())); + i = end; continue; } if (c == '/' && i + 1 < fragment.length()) { @@ -1127,27 +1164,19 @@ private static String[] statements(String text) { ListThis pins existing behaviour rather than verifying a fix: it was + * reported as broken, and reverting the change it prompted leaves this + * passing, because the configuration predicate accepts every main + * configuration whatever it is handed. Kept because the behaviour is worth + * holding, and labelled so nobody reads it as proof of something it does + * not test.
+ */ + @Test + public void aRuntimeOnlyPreMergePinSuppressesBoth() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " runtimeOnly 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'\n"); + check("".equals(out), + "a runtimeOnly pre-merge pin takes the sibling constraint with it"); + } + + /** + * The strict bypass reads both spellings. Asking only about the strictly + * keyword let a !! pin on a variant configuration be filtered out as a + * variant declaration and get the constraint anyway -- against a strict + * requirement that resolved fine before it. + */ + @Test + public void aShorthandPinOnAVariantIsStillStrict() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " debugImplementation " + + "'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22!!'\n"); + check("".equals(out), + "a !! pin on a variant configuration is honoured like a strictly call"); + + // and a variant declaration that is NOT strict still does not suppress + String plain = KotlinStdlibAlignment.constraintsBlock("implementation", + " debugImplementation " + + "'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.22'\n"); + check(plain.contains("kotlin-stdlib-jdk8:1.8.0"), + "a plain variant declaration still does not suppress"); + } + /** * A known definition referred to as $name inside a double-quoted string is * the same one hop already followed for a bare token. Reading it as From 1d2df8aba786f0a5fbdbe11228c39afd69382747 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:06:30 +0300 Subject: [PATCH 29/94] Read a literal's real delimiters, its position, and the script it belongs to Three findings, each one a place where reading part of a declaration gave the wrong answer and the block was suppressed for an app that had pinned nothing. A reason can be nothing BUT a coordinate. The whitespace rule separates prose from notation and could not see this one: `because 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'` names the artifact it is warning about, has no whitespace, and read as a declaration -- so the comment describing the duplicate switched off the constraint that prevents it. A literal in a reason's argument position is not a dependency, whatever it contains. A triple-quoted literal's content is not what you get by stripping one character from each end. Every caller was doing exactly that, so a coordinate written with the long delimiter kept two quotes at each end, had no readable version, and was classified pre-merge. One helper knows the delimiter length now and every reader goes through it. And the fragments are one script. They arrive as separate build hints but the builder concatenates them into a single generated build.gradle, so a `def` in android.gradlePlugin is in scope for a use in android.gradleDep. Reading them apart lost the definition at the boundary and the strict pin behind it went unseen -- the failure this must never produce, since a constraint cannot coexist with a strict version. Three new cases, each failing with its own fix reverted, checked one at a time rather than together: reverting all three at once reported three failures without saying which case belonged to which, which is not the same evidence. Co-Authored-By: Claude Opus 5 (1M context)A reason is usually prose and the whitespace rule catches it, but a + * reason can be nothing BUT a coordinate -- {@code because + * 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'} names the artifact it + * is warning about and has no whitespace at all. Read as a declaration, it + * supplied a pre-merge version and suppressed the entire block: the + * comment describing the duplicate switched off the constraint that + * prevents it.
+ */ + private static boolean isReasonArgument(String line, int quoteAt) { + int i = quoteAt - 1; + while (i >= 0 && (line.charAt(i) == ' ' || line.charAt(i) == '\t' + || line.charAt(i) == '(')) { + i--; + } + int end = i + 1; + while (i >= 0 && isIdentifierChar(line.charAt(i))) { + i--; + } + return end > i + 1 && BECAUSE.equals(line.substring(i + 1, end)); + } + + private static final String BECAUSE = "because"; + + /** + * The app's fragments as the one script they become. + * + *They are separate build hints but the builder concatenates them into + * a single generated {@code build.gradle}, so a {@code def} written in one + * is in scope for the next. Reading them apart lost the definition at the + * boundary and a strict pin behind it went unseen -- which is the failure + * this must never produce, since a constraint cannot coexist with a strict + * version.
+ */ + private static String combined(String[] appGradleFragments) { + if (appGradleFragments == null) { + return ""; + } + StringBuilder out = new StringBuilder(); + for (int i = 0; i < appGradleFragments.length; i++) { + if (appGradleFragments[i] == null) { + continue; + } + if (out.length() > 0) { + out.append('\n'); + } + out.append(appGradleFragments[i]); + } + return out.toString(); + } + /** Whether the text contains any whitespace. */ private static boolean hasWhitespace(String text) { for (int i = 0; i < text.length(); i++) { @@ -744,11 +793,8 @@ private static int parseSegment(String segment) { */ private static boolean declaresArtifact(String artifact, String configuration, String[] appGradleFragments) { - if (appGradleFragments == null) { - return false; - } - for (int i = 0; i < appGradleFragments.length; i++) { - String[] lines = activeLines(appGradleFragments[i]); + String[] lines = activeLines(combined(appGradleFragments)); + { for (int j = 0; j < lines.length; j++) { if (declaresArtifactOnLine(artifact, configuration, lines[j])) { return true; @@ -875,6 +921,32 @@ private static boolean isIdentifierChar(char c) { * brace counter that did the same swallowed a declaration's closing brace. * They call this now, so a fix reaches all of them. */ + /** + * The content of the literal opening at {@code quoteAt}, without its + * delimiters. + * + *Stripping one character from each end is wrong for a triple-quoted + * literal, and every caller was doing exactly that: a coordinate written + * with the long delimiter came back still wearing two quotes at each end, + * so it had no readable version and the declaration was classified + * pre-merge -- taking the whole block with it.
+ */ + private static String stringLiteralContent(String text, int quoteAt) { + int end = endOfStringLiteral(text, quoteAt); + int delimiter = delimiterLength(text, quoteAt); + int from = Math.min(quoteAt + delimiter, text.length()); + int to = Math.max(from, Math.min(end + 1 - delimiter, text.length())); + return text.substring(from, to); + } + + /** 3 for a triple-quoted literal, 1 otherwise. */ + private static int delimiterLength(String text, int quoteAt) { + char quote = text.charAt(quoteAt); + return quoteAt + 2 < text.length() + && text.charAt(quoteAt + 1) == quote + && text.charAt(quoteAt + 2) == quote ? 3 : 1; + } + private static int endOfStringLiteral(String text, int quoteAt) { char quote = text.charAt(quoteAt); // Groovy's triple-quoted literals are a different delimiter, not three of @@ -1463,8 +1535,7 @@ private static int trailingBraceBalance(String statement) { continue; } int end = endOfStringLiteral(statement, i); - if (statement.substring(i + 1, Math.min(end, statement.length())) - .startsWith(KOTLIN_GROUP)) { + if (stringLiteralContent(statement, i).startsWith(KOTLIN_GROUP)) { return braceBalance(statement.substring(Math.min(end + 1, statement.length()))); } diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index c3187313fb1..350b75020e9 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -457,6 +457,53 @@ public void aDefReferenceWithAMultilineClosureIsStillAPin() { "the strict base pin behind a def with a multiline closure is honoured"); } + /** + * A reason can be nothing BUT a coordinate, so the whitespace rule does not + * catch it. `because 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'` + * names the artifact it warns about; read as a declaration it supplied a + * pre-merge version and suppressed the whole block -- the comment + * describing the duplicate switching off the constraint that prevents it. + */ + @Test + public void aReasonThatIsOnlyACoordinateIsStillAReason() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('com.example:other:1.0') " + + "{ because 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22' }\n"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), + "a bare-coordinate reason does not declare anything"); + check(out.contains("kotlin-stdlib-jdk7:1.8.0"), + "and does not take the block with it"); + } + + /** + * A triple-quoted coordinate keeps its version. Stripping one character + * per side left the long delimiter's extra quotes on the content, so the + * version was unreadable and the declaration read as pre-merge. + */ + @Test + public void aTripleQuotedCoordinateKeepsItsVersion() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation '''org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.9.22'''\n"); + check(out.contains("kotlin-stdlib-jdk8:1.8.0"), + "a merged-era triple-quoted declaration leaves the sibling constrained"); + check(!out.contains("kotlin-stdlib-jdk7:1.8.0"), + "and pins its own artifact"); + } + + /** + * The fragments are separate build hints but one generated script, so a + * def written in one is in scope for the next. Reading them apart lost the + * definition at the boundary and the strict pin behind it went unseen. + */ + @Test + public void aDefinitionCrossesTheFragmentBoundary() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " def stdlib = 'org.jetbrains.kotlin:kotlin-stdlib:1.7.22'\n", + " implementation(stdlib) { version { strictly '1.7.22' } }\n"); + check("".equals(out), + "a definition in one fragment reaches a use in the next"); + } + /** * A triple-quoted literal is a different delimiter, not three of the same * one. Reading its opener as a single quote made it close on the first From ca4de85d1c0c4e92e8a5943a3f316fcfda1b476b Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:56:41 +0300 Subject: [PATCH 30/94] Sweep the spelling space, and stop the alignment from ever failing a build The review was finding this class one example at a time, and the examples kept being the same defect in a different place. Two changes of shape, then the individual fixes. Sweeping instead of exemplifying. Every equivalent spelling of a strict pre-merge pin is now asserted to suppress the block -- artifact, delimiter, configuration, call form, definition, decoration -- and the sweep immediately found a case no review comment had: a triple-quoted definition expanded to ""1.7.22"", no version parsed out of it, and the constraint written beside a live strict pin. That one does not fail the build; it resolves quietly to the empty shims and throws NoClassDefFoundError on the device. The one-character delimiter assumption behind it was live in three more places -- the map value, the interpolation, and a second hand-rolled quote scanner in declaresOn -- all now on the shared rule. The complementary sweep is asserted too: ordinary project text still gets both constraints, which is the direction that fails in silence. Never failing a build. The scanner reads developer-authored Groovy on every AndroidX build there is, to decide something that is an optimisation over a build which already worked apart from one duplicate class. A defect in it would not cost one app its constraint, it would cost every app its build. The call is now guarded so its worst case is "emit nothing", which is the behaviour before this feature existed, and the guard is pinned by a test. The individual findings: fragment order now matches the order the generated script emits them, since a definition is only in scope for what follows it; prefer is no longer read as a lower bound, and a declaration that binds no version no longer stands in for the constraint; typed locals declare a coordinate as much as def does; and Groovy's dollar-slashy literal no longer reads as a line comment. Plain slashy is deliberately still unhandled -- a lone slash is also division and both comment starts -- with the reasoning in the code. Declined, with the reasoning in the code: exempting strict pins on detached configurations. The Gradle fact is right, but acting on it means deciding from a configuration's NAME whether it shares a classpath with the constrained one, the names are open-ended, and "does not extend implementation" is not the same question as "cannot conflict". Getting that wrong is not symmetric. Co-Authored-By: Claude Opus 5 (1M context){@code prefer} is deliberately NOT read here. A preference is soft: + * Gradle takes it only when nothing stronger is in play, so a transitive + * requirement for a pre-merge shim beats it and the class-bearing jar wins + * anyway. Treating a preference as proof the artifact cannot resolve below + * the floor suppressed the constraint that was the only thing standing + * between that graph and the duplicate.
*/ private static String richVersionIn(String statement) { String strict = versionInCall(statement, STRICTLY); if (strict != null) { return strict; } - String required = versionInCall(statement, "require"); - if (required != null) { - return required; - } - return versionInCall(statement, "prefer"); + return versionInCall(statement, "require"); } /** The quoted argument of {@code call}, found outside string literals. */ @@ -634,7 +648,12 @@ private static String versionInCall(String statement, String call) { || statement.charAt(after) == '"')) { int end = endOfStringLiteral(statement, after); if (end < statement.length()) { - return statement.substring(after + 1, end); + // The literal's own delimiters, however many it has. Written + // strictly """1.7.22""", the one-per-side slice returned + // ""1.7.22"" -- which parsed as no version at all and only + // reached the right answer because an unreadable version counts + // as below the floor. Correct by accident is not correct. + return stringLiteralContent(statement, after); } } i = after; @@ -833,10 +852,39 @@ private static boolean declaresArtifactOnLine(String artifact, String configurat // fails the whole script at evaluation, which is a far larger blast radius // than the case it fixes. Revisit only with a project that actually has this // shape. + // + // Reviewed a third time with a sharper argument: a pin on a DETACHED + // configuration -- annotationProcessor, kapt, ksp -- genuinely cannot + // conflict, because unlike the variant configurations those do not extend + // implementation, so suppressing on one leaves the release runtime graph + // unaligned for nothing. The Gradle fact is right. What it asks for is not + // available here: acting on it means deciding from a configuration's NAME + // whether it shares a classpath with the one being constrained, and + // - Android synthesises a configuration per build type and flavour, so the + // names are open-ended: debugAnnotationProcessor, freeReleaseImplementation, + // and whatever the next plugin adds, + // - "does not extend implementation" is not the same question as "cannot + // conflict": compileOnly does not extend it either, yet compileClasspath + // extends both, so a strict pin there does conflict. + // A name list that gets this wrong is not wrong symmetrically. Classifying a + // conflicting configuration as detached emits the constraint beside a live + // strict pin, which is measured to fail resolution outright -- and for the + // `1.7.22!!` spelling to resolve quietly to the empty shims and throw + // NoClassDefFoundError on the device instead. Classifying a detached one as + // conflicting costs an app that had already pinned the family the duplicate + // it already had. So this stays until the classification can be read from + // something better than a name. if (!holdsStrictly(line, artifact) && !declaresOnTheConstrainedConfiguration(configuration, line)) { return false; } + if (!bindsAVersion(line, artifact)) { + // A declaration that pins nothing cannot stand in for the constraint. The + // clearest case is a lone preference: our floor overrides it, so emitting + // is harmless, while suppressing leaves a transitive pre-merge shim free + // to win. A declaration with no version at all is the same argument. + return false; + } if (namesCoordinate(line, artifact)) { return true; } @@ -1052,31 +1100,26 @@ private static boolean declaresOnTheConstrainedConfiguration(String configuratio /** Whether this line declares on {@code configuration}, as a whole token. */ private static boolean declaresOn(String configuration, String line) { - char quote = 0; - int stringStart = -1; for (int i = 0; i < line.length(); i++) { char c = line.charAt(i); - if (quote != 0) { - if (c == '\\' && i + 1 < line.length()) { - i++; - } else if (c == quote) { - // A configuration name inside a string counts in one place only: - // as the first argument of dependencies.add("runtimeOnly", ".."). - // Accepting any quoted occurrence read the word in a reason -- - // because 'implementation workaround' -- as a main-variant - // declaration, which suppressed the constraint for a dependency - // that only affects debug. - if (line.substring(stringStart + 1, i).equals(configuration) - && isAddCallArgument(line, stringStart)) { - return true; - } - quote = 0; - } - continue; - } if (c == '\'' || c == '"') { - quote = c; - stringStart = i; + // The shared rule rather than a third hand-rolled quote scanner. This + // one tracked a single delimiter character, so a triple-quoted name + // was read as an empty string followed by unquoted text -- the same + // defect that was live in the map-value and interpolation paths. + int end = endOfStringLiteral(line, i); + // A configuration name inside a string counts in one place only: + // as the first argument of dependencies.add("runtimeOnly", ".."). + // Accepting any quoted occurrence read the word in a reason -- + // because 'implementation workaround' -- as a main-variant + // declaration, which suppressed the constraint for a dependency + // that only affects debug. + if (end < line.length() + && stringLiteralContent(line, i).equals(configuration) + && isAddCallArgument(line, i)) { + return true; + } + i = end; continue; } if (line.startsWith(configuration, i)) { @@ -1153,6 +1196,24 @@ private static String[] activeLines(String fragment) { } continue; } + if (c == '$' && i + 1 < fragment.length() && fragment.charAt(i + 1) == '/') { + // Groovy's dollar-slashy literal. Its opener is unambiguous, and its + // content may start with a slash -- $/ /* /$ -- which the comment + // scanner below read as a line comment and used to discard the rest + // of the fragment, strict pin included. + // + // The PLAIN slashy form, /.../, is deliberately not recognised: a lone + // slash is also division and the start of both comment kinds, so + // telling them apart needs to know whether an expression is expected + // here, which is parsing rather than scanning. Guessing wrong there + // would swallow ordinary text, which is the failure this whole method + // exists to avoid. + int end = fragment.indexOf("/$", i + 2); + int stop = end < 0 ? fragment.length() : end + 2; + out.append(fragment, i, stop); + i = stop - 1; + continue; + } if (c == '\'' || c == '"') { // The shared rule, so triple-quoted literals and escapes are the // same thing here as everywhere else. This scanner and the statement @@ -1389,6 +1450,20 @@ private static void updateLiteralDefinitions(String statement, i = skipBlanks(statement, at + DEF.length()); } else { i = skipBlanks(statement, 0); + // A typed local declares just as much as def does: `String dep = '...'`. + // Two identifiers before the '=' is a declaration, one is an assignment, + // and only the first token differs. + int typeEnd = i; + while (typeEnd < statement.length() + && isIdentifierChar(statement.charAt(typeEnd))) { + typeEnd++; + } + int afterType = skipBlanks(statement, typeEnd); + if (typeEnd > i && afterType < statement.length() + && isIdentifierChar(statement.charAt(afterType))) { + declared = true; + i = afterType; + } } int nameStart = i; while (i < statement.length() && isIdentifierChar(statement.charAt(i))) { @@ -1485,8 +1560,15 @@ && isIdentifierChar(literal.charAt(nameEnd))) { out.append(c); continue; } - // Stored with its quotes, which do not belong inside another string. - out.append(value, 1, value.length() - 1); + // Stored with its quotes, which do not belong inside another string -- + // and with however many of them the literal was written with. Stripping + // one per side left a triple-quoted definition expanding to ""1.7.22"", + // no version parsed out of it, and the constraint written beside a + // strict pre-merge pin: the one outcome that fails at RUNTIME rather + // than in the build. Found by sweeping equivalent spellings of a strict + // pin, not by reading this line; the same one-character assumption was + // live in two other places. + out.append(stringLiteralContent(value, 0)); i = braced ? nameEnd : nameEnd - 1; } return out.toString(); diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index 350b75020e9..bd31728cfc2 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -457,6 +457,299 @@ public void aDefReferenceWithAMultilineClosureIsStillAPin() { "the strict base pin behind a def with a multiline closure is honoured"); } + /** + * The alignment can never fail a build. It reads developer-authored Groovy + * with a hand-written scanner, on every AndroidX build there is, to decide + * something that is an optimisation over a build which already worked -- + * so an index defect in it must cost that one app its constraint, not + * every app its build. The guard is asserted here rather than trusted, + * because nothing else in the suite would notice it being refactored away. + */ + @Test + public void theAlignmentCannotFailTheBuild() throws Exception { + byte[] bytes = java.nio.file.Files.readAllBytes(new java.io.File( + "src/main/java/com/codename1/builders/AndroidGradleBuilder.java").toPath()); + String builderSrc = new String(bytes, "UTF-8"); + int at = builderSrc.indexOf("KotlinStdlibAlignment.constraintsBlock("); + check(at >= 0, "the builder calls the alignment"); + String before = builderSrc.substring(0, at); + check(before.lastIndexOf("try {") > before.lastIndexOf("catch ("), + "the call is inside a try block"); + String after = builderSrc.substring(at); + int handler = after.indexOf("catch (RuntimeException"); + check(handler >= 0, "and a RuntimeException handler follows it"); + // Past the handler's own reasoning, which is longer than the code. + String body = after.substring(handler, + Math.min(handler + 2000, after.length())); + check(body.indexOf("kotlinStdlibConstraints = \"\"") >= 0, + "which falls back to emitting nothing"); + check(body.indexOf("log(") >= 0, + "and says so, rather than swallowing the defect"); + } + + /** + * How a literal is delimited changes nothing about what it says, so the + * same declaration written four ways produces the same block. Asserted as + * an equivalence rather than case by case because the strict sweep already + * passed the triple-quoted spelling for the wrong reason: the version came + * back as {@code ""1.9.22""}, no version parsed out of it, and an + * unreadable version counts as below the floor -- which happens to be the + * safe answer, so nothing failed while the read was wrong. + */ + @Test + public void theDelimiterDoesNotChangeWhatADeclarationSays() { + String[] quotes = {"'", "\"", "'''", "\"\"\""}; + String[] shapes = { + "implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8') " + + "{ version { strictly %s1.9.22%s } }", + "implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8') " + + "{ version { require %s1.9.22%s } }", + "implementation group: %sorg.jetbrains.kotlin%s, " + + "name: %skotlin-stdlib-jdk8%s, version: '1.9.22'", + "dependencies.add(%simplementation%s, " + + "'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22')", + }; + for (int s = 0; s < shapes.length; s++) { + String expected = null; + for (int q = 0; q < quotes.length; q++) { + String text = shapes[s].replace("%s", quotes[q]); + String out = KotlinStdlibAlignment.constraintsBlock("implementation", text); + if (expected == null) { + expected = out; + continue; + } + check(expected.equals(out), + "the delimiter does not change the answer for <<" + text + + ">>: expected <<" + expected + ">> got <<" + out + ">>"); + } + } + } + + /** + * A pre-merge shim added through {@code dependencies.add} suppresses the + * block, whichever delimiter names the configuration. Emitting beside it + * raises kotlin-stdlib past the app's own class-bearing 1.7.22 jar, which + * is this block manufacturing the duplicate it exists to prevent. + */ + @Test + public void anAddedPreMergeShimSuppressesTheBlock() { + String[] quotes = {"'", "\"", "'''", "\"\"\""}; + for (int q = 0; q < quotes.length; q++) { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " dependencies.add(" + quotes[q] + "implementation" + quotes[q] + + ", 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22')\n"); + check("".equals(out), + "an added pre-merge shim suppresses the block, named with " + + quotes[q] + " but got <<" + out + ">>"); + } + } + + /** + * The complement of the sweep below, and the direction that fails in + * silence: an app whose Gradle text says nothing about Kotlin still gets + * both constraints. Every recognition rule added to this class is a new + * way to conclude "the app has this covered", and concluding it wrongly + * does not fail anything -- it just hands the duplicate class back to the + * app this whole change exists to fix, with no signal anywhere. + */ + @Test + public void ordinaryProjectTextStillGetsTheAlignment() { + String[] ordinary = { + "implementation 'androidx.appcompat:appcompat:1.6.1'", + "implementation('com.android.billingclient:billing:9.1.0')", + "implementation group: 'com.google.android.material', name: 'material', " + + "version: '1.11.0'", + "def v = '1.7.22'\nimplementation(\"com.squareup.okhttp3:okhttp:$v\")", + "annotationProcessor 'com.github.bumptech.glide:compiler:4.16.0'", + "implementation fileTree(dir: 'libs', include: ['*.jar'])", + // Commented out is not declared, in either comment syntax. + "// implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'", + "/* implementation 'org.jetbrains.kotlin:kotlin-stdlib:1.7.22!!' */", + // And a reason string is prose, not a pin. + "implementation('a:b:1.0') { because 'strictly 1.7.22 was never wanted' }", + "testImplementation 'junit:junit:4.13.2'", + "", + }; + String[] decorations = { + "%s", " %s", "dependencies {\n%s\n}", "%s // note", + "%s\nimplementation 'com.google.code.gson:gson:2.10.1'", + }; + for (int o = 0; o < ordinary.length; o++) { + for (int d = 0; d < decorations.length; d++) { + String text = decorations[d].replace("%s", ordinary[o]); + String out = KotlinStdlibAlignment.constraintsBlock("implementation", text); + check(out.contains("kotlin-stdlib-jdk7:1.8.0") + && out.contains("kotlin-stdlib-jdk8:1.8.0"), + "both constraints are still written for <<" + text + + ">> but got <<" + out + ">>"); + } + } + } + + /** + * Every equivalent way of writing a strict pre-merge pin suppresses the + * block. This is a sweep rather than an example, because the examples were + * being found one review comment at a time while the same defect sat in + * three different places: a triple-quoted definition expanded to + * {@code ""1.7.22""}, no version was parsed out of it, and the constraint + * went in beside the strict pin -- which does not fail the build, it + * silently strips the classes and throws NoClassDefFoundError on the + * device. That is the one outcome this class must never produce, so the + * property is asserted over the whole spelling space and not over the + * spellings somebody happened to think of. + */ + @Test + public void everySpellingOfAStrictPreMergePinSuppressesTheBlock() { + String[] artifacts = {"kotlin-stdlib", "kotlin-stdlib-jdk7", "kotlin-stdlib-jdk8"}; + String[] quotes = {"'", "\"", "'''", "\"\"\""}; + String[] configurations = {"implementation", "api", "compile"}; + int checked = 0; + for (int a = 0; a < artifacts.length; a++) { + String coordinate = "org.jetbrains.kotlin:" + artifacts[a]; + for (int q = 0; q < quotes.length; q++) { + String u = quotes[q]; + for (int c = 0; c < configurations.length; c++) { + String on = configurations[c]; + String[] forms = { + on + "(" + u + coordinate + ":1.7.22!!" + u + ")", + on + " " + u + coordinate + ":1.7.22!!" + u, + on + "(" + u + coordinate + u + ") { version { strictly " + + u + "1.7.22" + u + " } }", + on + "(" + u + coordinate + u + ")\n{ version { strictly " + + u + "1.7.22" + u + " } }", + "def v = " + u + "1.7.22" + u + "\n" + on + "(\"" + + coordinate + ":$v!!\")", + "def d = " + u + coordinate + ":1.7.22!!" + u + "\n" + on + "(d)", + "String d = " + u + coordinate + ":1.7.22!!" + u + "; " + on + "(d)", + }; + for (int f = 0; f < forms.length; f++) { + String[] decorated = { + forms[f], + " " + forms[f], + "\t" + forms[f] + " ", + forms[f] + " // a note", + "/* lead */ " + forms[f], + "dependencies {\n" + forms[f] + "\n}", + "repositories { mavenCentral() }\n" + forms[f], + forms[f] + "\nimplementation 'androidx.appcompat:appcompat:1.6.1'", + "implementation 'androidx.appcompat:appcompat:1.6.1'\n" + forms[f], + }; + for (int d = 0; d < decorated.length; d++) { + checked++; + String out = KotlinStdlibAlignment.constraintsBlock( + "implementation", decorated[d]); + check("".equals(out), + "a strict pre-merge pin suppresses the block, written as <<" + + decorated[d] + ">> but got <<" + out + ">>"); + } + } + } + } + } + check(checked > 2000, "the sweep really ran over the matrix: " + checked); + } + + /** + * A preference is soft: Gradle takes it only when nothing stronger is in + * play, so a transitive requirement for a pre-merge shim beats it. Reading + * one as proof the artifact cannot resolve below the floor suppressed the + * constraint that was the only thing standing between that graph and the + * duplicate. + */ + @Test + public void aPreferenceDoesNotStandInForTheConstraint() { + String modern = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8') " + + "{ version { prefer '1.9.22' } }\n"); + check(modern.contains("kotlin-stdlib-jdk8:1.8.0"), + "a preferred version does not suppress the constraint"); + + String old = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8') " + + "{ version { prefer '1.7.22' } }\n"); + check(old.contains("kotlin-stdlib-jdk8:1.8.0"), + "and neither does an old one, which the floor simply overrides"); + + // a required version still does + String required = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8') " + + "{ version { require '1.9.22' } }\n"); + check(!required.contains("kotlin-stdlib-jdk8:1.8.0"), + "a required version still binds"); + } + + /** + * A map value written with the long delimiter keeps its content. Stripping + * one character per side left the group and name wearing two quotes, so + * both failed their exact match and the declaration was ignored. + */ + @Test + public void aTripleQuotedMapValueKeepsItsContent() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation group: '''org.jetbrains.kotlin''', " + + "name: '''kotlin-stdlib-jdk8''', version: '1.9.22'\n"); + check(!out.contains("kotlin-stdlib-jdk8:1.8.0"), + "a triple-quoted map declaration still pins its artifact"); + } + + /** + * A typed local declares as much as def does. + */ + @Test + public void aTypedLocalDefinesACoordinate() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " String dep = 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'\n" + + " implementation(dep) { version { strictly '1.7.22' } }\n"); + check("".equals(out), "a typed local carries the coordinate too"); + } + + /** + * Groovy's dollar-slashy literal may open with a slash, which the comment + * scanner read as a line comment and used to discard the rest of the + * fragment, strict pin included. The PLAIN slashy form is deliberately not + * recognised -- a lone slash is also division and both comment kinds -- + * and that limit has its own assertion below. + */ + @Test + public void aDollarSlashyLiteralDoesNotOpenAComment() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + // On one line, because a line comment only reaches the end of its + // own line: put the pin on the next one and the test passes with + // the literal unrecognised, which proves nothing. + " def marker = $//*/$; " + + "implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22') " + + "{ version { strictly '1.7.22' } }\n"); + check("".equals(out), + "the strict pin after a dollar-slashy literal is still seen"); + } + + /** + * The builder hands the fragments over in the order the generated script + * emits them, because a definition is only in scope for what follows it. + */ + @Test + public void theBuilderPassesFragmentsInGeneratedOrder() throws Exception { + byte[] bytes = java.nio.file.Files.readAllBytes(new java.io.File( + "src/main/java/com/codename1/builders/AndroidGradleBuilder.java").toPath()); + String builderSrc = new String(bytes, "UTF-8"); + int at = builderSrc.indexOf("KotlinStdlibAlignment.constraintsBlock("); + check(at >= 0, "the builder calls the alignment"); + String call = builderSrc.substring(at, builderSrc.indexOf(";", at)); + // The call carries a comment naming those same hints, in an order that + // has nothing to do with the arguments -- read the arguments only. + call = call.replaceAll("//[^\n]*", ""); + int plugin = call.indexOf("getArg(\"android.gradlePlugin\""); + int support = call.indexOf("getArg(\"android.supportv4Dep\""); + int additional = call.indexOf("additionalDependencies"); + int dep = call.indexOf("getArg(\"android.gradleDep\""); + int xgradle = call.indexOf("getArg(\"android.xgradle\""); + check(plugin >= 0 && support >= 0 && additional >= 0 && dep >= 0 && xgradle >= 0, + "every app-controlled fragment is passed"); + check(plugin < support && support < additional && additional < dep + && dep < xgradle, + "and in the order the generated script emits them"); + } + /** * A reason can be nothing BUT a coordinate, so the whitespace rule does not * catch it. `because 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'` From c0513a766ba5b8ce3467be7b0c4253fea9a10040 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 20:18:20 +0300 Subject: [PATCH 31/94] Give the last scanner the shared rule, and stop reading a strict pin as binding nothing Five findings, four of them the same two shapes this class keeps producing: a rule written out twice, and a literal read as if its delimiter were one character. The one that was mine. Last round's bindsAVersion guard -- a declaration that pins nothing cannot stand in for the constraint -- also rejected a pin whose version this cannot evaluate. `strictly kotlinVersion` takes its version from a property, so the guard read a live strict pin as binding nothing and emitted the constraint next to it. That is the direction that does not fail the build: it resolves to the empty shims and throws NoClassDefFoundError on the device. A strict pin is now exempt from the guard, readable or not, which is the conservative fall-back belowTheFloor already takes for the same reason. The rule written twice. activeLines learned Groovy's dollar-slashy literal last round and statements() did not, so an apostrophe inside $/can't/$ still opened a string there and swallowed the pin behind it. And namesCoordinate was the last scanner still tracking a delimiter character of its own: it closed a triple-quoted literal on the second of the three and read the third as a new opener, so a reason written '''...''' lost its `because` and was taken for the declaration it was warning about. Every scanner in the file now uses endOfStringLiteral; there are no hand-rolled ones left. The rest. A typed local is written with however many modifiers the author felt like, so `final String dep` is a declaration and counting exactly two tokens read `String` as its name. And ext.kotlinVersion = '1.9.22' really does bind the bare name the interpolation reads, which is how a project-wide Kotlin version is nearly always written -- restricted to that one prefix on purpose, since recording any dotted assignment would let an unrelated property supply a version it does not bind and turn an unreadable version into a confidently wrong one. The sweep grew the new spellings, and three of its definition forms turned out to be proving nothing: written `def d = 'coord:1.7.22!!'`, that line names the artifact and ends in !!, so it suppressed on its own whether or not the name was ever carried to the usage. They now carry the coordinate and nothing else, with the strict pin on the usage, which is what makes the inlining the thing under test. Co-Authored-By: Claude Opus 5 (1M context)This question is asked in eleven places, and the answer used to be + * spelled out at each of them as "a quote is here". Every literal form + * added since arrived as a review comment against one of those eleven -- + * triple quotes, then dollar-slashy in the comment scanner, then + * dollar-slashy in the statement scanner, then dollar-slashy in the + * coordinate matcher -- because teaching one site never taught the rest. + * The form belongs here, once, where every scanner reads it.
+ */ + private static boolean isLiteralStart(String text, int at) { + char c = text.charAt(at); + if (c == '\'' || c == '"') { + return true; + } + if (c == '$') { + return at + 1 < text.length() && text.charAt(at + 1) == '/'; + } + return c == '/' && opensASlashyLiteral(text, at); + } + + /** + * Whether a {@code /} at {@code at} opens a slashy literal rather than + * dividing or opening a comment. + * + *Declined once, on the grounds that telling these apart needs to know + * whether an expression is expected here, which is parsing rather than + * scanning. That was raised again with a better argument: NOT recognizing + * the literal fails in the SAME direction as recognizing one that is not + * there -- an apostrophe inside {@code /can't/} puts the quote scanner out + * of step and hides whatever follows, exactly as swallowing a division + * would. Given both mistakes cost the same, the question is only which is + * likelier, and that is decidable: a literal can only open where an + * expression may begin. After an identifier, a number or a closing + * bracket -- which is every division a build script actually contains, + * {@code total / 2}, {@code (a + b) / 2} -- it is division. The two + * comment openers are excluded outright.
+ */ + private static boolean opensASlashyLiteral(String text, int at) { + if (at + 1 < text.length() + && (text.charAt(at + 1) == '/' || text.charAt(at + 1) == '*')) { + return false; + } + int i = at - 1; + while (i >= 0 && (text.charAt(i) == ' ' || text.charAt(i) == '\t' + || text.charAt(i) == '\r' || text.charAt(i) == '\n')) { + i--; + } + if (i < 0) { + return true; + } + return SLASHY_OPENER_POSITIONS.indexOf(text.charAt(i)) >= 0; + } + + /** + * The characters an expression may follow. Deliberately does not include + * an identifier character, a digit or a closing bracket: those are what + * division follows. + */ + private static final String SLASHY_OPENER_POSITIONS = "=(,[:{&|!?+-*;"; + + /** The length of the delimiter opening at {@code at}. */ private static int delimiterLength(String text, int quoteAt) { char quote = text.charAt(quoteAt); + if (quote == '$') { + return 2; + } + if (quote == '/') { + return 1; + } return quoteAt + 2 < text.length() && text.charAt(quoteAt + 1) == quote && text.charAt(quoteAt + 2) == quote ? 3 : 1; @@ -1000,6 +1068,28 @@ private static int delimiterLength(String text, int quoteAt) { private static int endOfStringLiteral(String text, int quoteAt) { char quote = text.charAt(quoteAt); + if (quote == '$') { + // $/ ... /$ -- the closer is two characters, and the content may hold + // anything at all, which is the point of the form. + int close = text.indexOf("/$", quoteAt + 2); + return close < 0 ? text.length() : close + 1; + } + if (quote == '/') { + for (int i = quoteAt + 1; i < text.length(); i++) { + char c = text.charAt(i); + if (c == '\\') { + i++; + } else if (c == '/') { + return i; + } else if (c == '\n') { + // A slashy literal does not cross a line; treating an unterminated + // one as running to the end of the fragment would swallow every + // statement after it. + return i - 1; + } + } + return text.length(); + } // Groovy's triple-quoted literals are a different delimiter, not three of // this one. Treating the opener as a single quote made a triple-quoted note // close on the first apostrophe it contains -- can't, in the case that found @@ -1105,7 +1195,7 @@ private static boolean declaresOnTheConstrainedConfiguration(String configuratio private static boolean declaresOn(String configuration, String line) { for (int i = 0; i < line.length(); i++) { char c = line.charAt(i); - if (c == '\'' || c == '"') { + if (isLiteralStart(line, i)) { // The shared rule rather than a third hand-rolled quote scanner. This // one tracked a single delimiter character, so a triple-quoted name // was read as an empty string followed by unquoted text -- the same @@ -1199,25 +1289,7 @@ private static String[] activeLines(String fragment) { } continue; } - if (c == '$' && i + 1 < fragment.length() && fragment.charAt(i + 1) == '/') { - // Groovy's dollar-slashy literal. Its opener is unambiguous, and its - // content may start with a slash -- $/ /* /$ -- which the comment - // scanner below read as a line comment and used to discard the rest - // of the fragment, strict pin included. - // - // The PLAIN slashy form, /.../, is deliberately not recognised: a lone - // slash is also division and the start of both comment kinds, so - // telling them apart needs to know whether an expression is expected - // here, which is parsing rather than scanning. Guessing wrong there - // would swallow ordinary text, which is the failure this whole method - // exists to avoid. - int end = fragment.indexOf("/$", i + 2); - int stop = end < 0 ? fragment.length() : end + 2; - out.append(fragment, i, stop); - i = stop - 1; - continue; - } - if (c == '\'' || c == '"') { + if (isLiteralStart(fragment, i)) { // The shared rule, so triple-quoted literals and escapes are the // same thing here as everywhere else. This scanner and the statement // scanner below kept their own copies through the consolidation, and @@ -1302,19 +1374,7 @@ private static String[] statements(String text) { int depth = 0; for (int i = 0; i < text.length(); i++) { char c = text.charAt(i); - if (c == '$' && i + 1 < text.length() && text.charAt(i + 1) == '/') { - // Groovy's dollar-slashy literal, recognised here as well as in the - // comment scanner. Fixing only that one left this scanner reading an - // apostrophe inside $/can't/$ as an opening quote, which swallowed - // the strict pin that followed on the same line. The same rule in two - // scanners is the shape this class keeps getting wrong. - int slashy = text.indexOf("/$", i + 2); - int stop = slashy < 0 ? text.length() : slashy + 2; - current.append(text, i, stop); - i = stop - 1; - continue; - } - if (c == '\'' || c == '"') { + if (isLiteralStart(text, i)) { // The shared rule: escapes and triple quotes handled in one place. // A literal that closed early here merged statements that must stay // apart, which lets one statement's configuration pair with another @@ -1437,12 +1497,27 @@ private static ListGradle's notation carries two optional modifiers after the version -- + * a classifier as a fourth colon-separated part, and an {@code @extension} + * -- and both were being returned as part of the version. That leaves + * {@code 1.7.22!!@jar}, which does not end in the strict marker, so a + * strict pre-merge pin read as an ordinary one and the constraint was + * written beside it.
+ */ + private static String versionComponentOf(String remainder) { + int end = remainder.length(); + int at = remainder.indexOf('@'); + if (at >= 0) { + end = at; + } + int classifier = remainder.indexOf(':'); + if (classifier >= 0 && classifier < end) { + end = classifier; + } + return remainder.substring(0, end); + } + private static String mapEntryValue(String line, String key) { for (int i = 0; i < line.length(); i++) { char c = line.charAt(i); @@ -1042,9 +1065,34 @@ private static boolean opensASlashyLiteral(String text, int at) { if (i < 0) { return true; } - return SLASHY_OPENER_POSITIONS.indexOf(text.charAt(i)) >= 0; + if (SLASHY_OPENER_POSITIONS.indexOf(text.charAt(i)) >= 0) { + return true; + } + // Division needs a VALUE on its left, and a keyword is not one. `return + // /can't/` is a literal for the same reason `= /can't/` is, so the test is + // not "is the previous character an identifier character" but "is the + // previous TOKEN something that can be divided". A word that is not a + // keyword is a variable, and dividing it is exactly what a build script + // does. + if (!isIdentifierChar(text.charAt(i))) { + return false; + } + int tokenEnd = i + 1; + while (i >= 0 && isIdentifierChar(text.charAt(i))) { + i--; + } + String token = text.substring(i + 1, tokenEnd); + return EXPRESSION_KEYWORDS.indexOf(" " + token + " ") >= 0; } + /** + * Groovy words after which an expression begins, so a slash is a literal + * rather than a division. Reserved words cannot be variables, which is why + * this can be read off the language rather than guessed at. + */ + private static final String EXPRESSION_KEYWORDS = + " return new in case else do while if throw assert yield instanceof "; + /** * The characters an expression may follow. Deliberately does not include * an identifier character, a digit or a closing bracket: those are what diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index 6c0ed47ef2e..dd2ba7506a6 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -457,6 +457,82 @@ public void aDefReferenceWithAMultilineClosureIsStillAPin() { "the strict base pin behind a def with a multiline closure is honoured"); } + /** + * Every fragment the generated dependencies block is built from is handed + * to the alignment, and in the same order. + * + *Read off the builder's own concatenation rather than listed here, + * because a list here is a second copy of the truth and it was already + * wrong: kotlinRuntimeDependency carries requireKotlinStdlib, so an app + * asking for {@code 1.7.22!!} had a strict pre-merge pin on the base + * library that nothing in the scan could see. Two more fragments were + * missing beside it. A test that enumerates cannot go stale the way the + * list did.
+ */ + @Test + public void everyFragmentOfTheGeneratedBlockIsScanned() throws Exception { + byte[] bytes = java.nio.file.Files.readAllBytes(new java.io.File( + "src/main/java/com/codename1/builders/AndroidGradleBuilder.java").toPath()); + String builderSrc = new String(bytes, "UTF-8"); + int at = builderSrc.indexOf("KotlinStdlibAlignment.constraintsBlock("); + check(at >= 0, "the builder calls the alignment"); + String call = builderSrc.substring(at, builderSrc.indexOf(";", at)) + .replaceAll("//[^\n]*", ""); + + int blockAt = builderSrc.indexOf("\"dependencies {"); + check(blockAt >= 0, "the generated dependencies block is found"); + int blockEnd = builderSrc.indexOf("+ \"}\\n\"", blockAt); + check(blockEnd > blockAt, "and its end"); + String block = builderSrc.substring(blockAt, blockEnd).replaceAll("//[^\n]*", ""); + + // What the block is concatenated FROM: java expressions, not literals. + // Keyed by where they appear, because the two spellings have to come back + // interleaved: collecting all the hints and then all the locals produced a + // list in neither the script's order nor the call's, and the order half of + // this test then failed on a call that was right. + java.util.TreeMapA force is as absolute as a strict pin and worse to get wrong. It + * does not conflict with a constraint, it silently wins: with + * {@code resolutionStrategy.force 'org.jetbrains.kotlin:kotlin-stdlib:1.7.22'} + * the base library stays pre-merge while these constraints raise the shims + * to their EMPTY 1.8.0 jars, so the jdk7/jdk8 classes end up in no selected + * jar at all. Nothing fails in the build; it throws on the device. So a + * forced version is read exactly like a strict one.
+ */ + private static boolean callsForce(String statement) { + return callsNamed(statement, FORCE); + } + + private static final String FORCE = "force"; + private static boolean callsStrictly(String statement) { + return callsNamed(statement, STRICTLY); + } + + /** Whether {@code call} appears as a call, rather than inside a literal. */ + private static boolean callsNamed(String statement, String call) { for (int i = 0; i < statement.length(); i++) { - char c = statement.charAt(i); if (isLiteralStart(statement, i)) { i = endOfStringLiteral(statement, i); continue; } - if (statement.startsWith(STRICTLY, i)) { + if (statement.startsWith(call, i)) { boolean startsToken = i == 0 || !isIdentifierChar(statement.charAt(i - 1)); - int after = i + STRICTLY.length(); + int after = i + call.length(); boolean endsToken = after < statement.length() && (statement.charAt(after) == ' ' || statement.charAt(after) == '(' @@ -1098,7 +1122,7 @@ private static boolean opensASlashyLiteral(String text, int at) { * an identifier character, a digit or a closing bracket: those are what * division follows. */ - private static final String SLASHY_OPENER_POSITIONS = "=(,[:{&|!?+-*;"; + private static final String SLASHY_OPENER_POSITIONS = "=(,[:{&|!?+-*;<>"; /** The length of the delimiter opening at {@code at}. */ private static int delimiterLength(String text, int quoteAt) { @@ -1123,20 +1147,24 @@ private static int endOfStringLiteral(String text, int quoteAt) { return close < 0 ? text.length() : close + 1; } if (quote == '/') { + // Groovy's slashy literals MAY span lines, so the closing slash is looked + // for across them -- but only a literal that actually closes gets to. An + // opener this misread, with no closing slash anywhere, would otherwise + // swallow every statement after it, and a suppression reached that way is + // the outcome this class must never produce. So: close where it closes, + // and failing that, stop at the line it started on. + int firstNewline = -1; for (int i = quoteAt + 1; i < text.length(); i++) { char c = text.charAt(i); if (c == '\\') { i++; } else if (c == '/') { return i; - } else if (c == '\n') { - // A slashy literal does not cross a line; treating an unterminated - // one as running to the end of the fragment would swallow every - // statement after it. - return i - 1; + } else if (c == '\n' && firstNewline < 0) { + firstNewline = i; } } - return text.length(); + return firstNewline < 0 ? text.length() : firstNewline - 1; } // Groovy's triple-quoted literals are a different delimiter, not three of // this one. Treating the opener as a single quote made a triple-quoted note diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index dd2ba7506a6..069e027ee2c 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -563,6 +563,79 @@ public void theAlignmentCannotFailTheBuild() throws Exception { "and says so, rather than swallowing the defect"); } + /** + * A forced version suppresses the block, like a strict one. + * + *A force does not conflict with a constraint, it wins over it without + * a word: force the base library to 1.7.22 and these constraints still + * raise the shims to their EMPTY 1.8.0 jars, so the jdk7/jdk8 classes are + * in no selected jar at all. The build is green and the app throws on the + * device, which is the one outcome worth all of this machinery.
+ */ + @Test + public void aForcedVersionIsHeldAsFirmlyAsAStrictOne() { + String[] forced = { + " configurations.all { resolutionStrategy.force " + + "'org.jetbrains.kotlin:kotlin-stdlib:1.7.22' }\n", + " configurations.all { resolutionStrategy { force " + + "'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22' } }\n", + " configurations.all {\n resolutionStrategy {\n" + + " force 'org.jetbrains.kotlin:kotlin-stdlib:1.7.22'\n" + + " }\n }\n", + }; + for (int i = 0; i < forced.length; i++) { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", forced[i]); + check("".equals(out), + "a forced pre-merge version suppresses the block, from <<" + + forced[i] + ">> got <<" + out + ">>"); + } + + // A force at or above the floor takes nothing away: it is already a shim. + String modern = KotlinStdlibAlignment.constraintsBlock("implementation", + " configurations.all { resolutionStrategy.force " + + "'org.jetbrains.kotlin:kotlin-stdlib:1.9.22' }\n"); + check(modern.contains("kotlin-stdlib-jdk8:1.8.0"), + "a merged-era force still gets the alignment, got <<" + modern + ">>"); + + // And the word in a reason is prose, as everywhere else. + String prose = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('com.example:other:1.0') " + + "{ because 'we force nothing here' }\n"); + check(prose.contains("kotlin-stdlib-jdk8:1.8.0"), + "the word force in prose is not a force, got <<" + prose + ">>"); + } + + /** + * A slashy literal may open after a closure arrow, and may run past the + * end of its line -- but only when it actually closes. An opener misread + * with no closing slash anywhere would swallow every statement after it, + * and a suppression reached that way says nothing about the app. + */ + @Test + public void aSlashyLiteralSpansLinesOnlyWhenItCloses() { + String afterArrow = KotlinStdlibAlignment.constraintsBlock("implementation", + " def marker = { -> /can't/ }; " + + "implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22') " + + "{ version { strictly '1.7.22' } }\n"); + check("".equals(afterArrow), + "a slashy literal after a closure arrow, got <<" + afterArrow + ">>"); + + String multiline = KotlinStdlibAlignment.constraintsBlock("implementation", + " def marker = /first\n still can't/\n" + + " implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22') " + + "{ version { strictly '1.7.22' } }\n"); + check("".equals(multiline), + "a literal that spans lines keeps its content, got <<" + multiline + ">>"); + + // Unterminated: whatever that slash was, it does not reach the next line. + String unterminated = KotlinStdlibAlignment.constraintsBlock("implementation", + " def ratio = a / b\n" + + " implementation 'androidx.appcompat:appcompat:1.6.1'\n"); + check(unterminated.contains("kotlin-stdlib-jdk7:1.8.0") + && unterminated.contains("kotlin-stdlib-jdk8:1.8.0"), + "an unclosed slash stops at its line, got <<" + unterminated + ">>"); + } + /** * A coordinate may carry a classifier and an {@code @extension} after its * version, and neither is part of the version. Returning them made From cf45c7a97c20c9ec446dc89a260f990f54f9c5c3 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 21:27:22 +0300 Subject: [PATCH 35/94] Ask which half is closed, and let a removed comment still separate its tokens The slashy rule stops growing. "Which characters may an expression follow" had been extended by review four times -- the closure arrow, the comparison, then Groovy's =~ and ==~ -- and was still missing whichever operator came next, because that half is open-ended. Division is the closed half: it needs a value on its left, and a value ends with an identifier that is not a keyword, a digit, a closing bracket, a string, or an increment. Everything else opens a literal, including every operator nobody has thought of yet, and the character set is deleted. The half the code no longer enumerates is enumerated in the test instead, which is where an open-ended list can live without being wrong: twenty-eight operators, each asserted to leave the pin behind it visible. The closed half has its own sweep, because that is the one where adding a case is how a division starts swallowing statements. Also: removing a block comment leaves the whitespace it was. A comment separates tokens in the language, so deleting it outright joined them -- strictly/* pin */'1.7.22' became strictly'1.7.22', which is not a call to strictly, and the strict pin behind it was never seen. Gradle accepts the original and records {strictly 1.7.22}. Co-Authored-By: Claude Opus 5 (1M context)Swept over the operators rather than asserted one at a time, because + * one at a time is how the rule was built and it took four review rounds + * to still be incomplete: the closure arrow, then the comparison, then + * Groovy's {@code =~} and {@code ==~}. The code no longer enumerates this + * half at all -- it enumerates the closed one, what a value can end with -- + * so this test is where the open half is written down.
+ */ + @Test + public void aSlashAfterAnythingThatIsNotAValueOpensALiteral() { + String[] operators = { + "=", "=~", "==~", "~", "->", ",", "(", "[", ":", "&&", "||", + "!", "?", "+", "-", "*", "%", "^", "|", "&", "<", ">", "<=", ">=", + "==", "!=", "<<", "?:", + }; + for (int i = 0; i < operators.length; i++) { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " def marker = [ name " + operators[i] + " /can't/ ]; " + + "implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22') " + + "{ version { strictly '1.7.22' } }\n"); + check("".equals(out), + "a slash after " + operators[i] + + " opens a literal, so the pin behind it is still seen; got <<" + + out + ">>"); + } + } + + /** + * And a slash after a value divides it. This is the half the code + * enumerates, so it is the half that must stay closed: adding to it is how + * a division starts swallowing the statements after it. + */ + @Test + public void aSlashAfterAValueDividesIt() { + String[] values = { + "total", "2", "count()", "sizes[0]", "(a + b)", "1.5", "n++", "n--", + }; + for (int i = 0; i < values.length; i++) { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " def ratio = " + values[i] + " / divisor; " + + "implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22') " + + "{ version { strictly '1.7.22' } }\n"); + check("".equals(out), + "dividing " + values[i] + + " does not swallow the pin after it; got <<" + out + ">>"); + } + } + /** * A forced version suppresses the block, like a strict one. * From d0959354fcd241dae8d72cc36024bfc869ae0533 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 21:38:44 +0300 Subject: [PATCH 36/94] Let a line ending, a blank line and an escaped delimiter all mean nothing Three findings, all of them the scanner disagreeing with Groovy about what is whitespace or what is a delimiter. Line endings. The token-end test accepted a space, a tab or an open parenthesis, so a fragment written on Windows put a carriage return after `strictly` and the call stopped being a call -- the strict pin behind it was never read. Spelled out in two places, both now asking isBlank, which is the whole set. Line endings are not this class's business to have an opinion about. Blank lines. A trailing closure still belongs to its call with a comment-only line between them; stripping the comment leaves an empty statement, and looking at only the very next statement left the closure, and the strict version inside it, attached to nothing. Escaped delimiters. Inside a dollar-slashy literal the dollar escapes itself and a slash, so $/ is a slash and not the closer. Taking the first "/$" substring ended the literal early and put the scanner back into code halfway through a string. Co-Authored-By: Claude Opus 5 (1M context)Spelled out as space-or-tab in two places, which meant a fragment with + * Windows line endings put a carriage return after {@code strictly} and the + * call stopped being a call. Line endings are not this class's business to + * have an opinion about.
+ */ + private static boolean isBlank(char c) { + return c == ' ' || c == '\t' || c == '\r' || c == '\n'; + } + private static boolean isIdentifierChar(char c) { return Character.isLetterOrDigit(c) || c == '_' || c == '$'; } @@ -1150,9 +1161,23 @@ private static int endOfStringLiteral(String text, int quoteAt) { char quote = text.charAt(quoteAt); if (quote == '$') { // $/ ... /$ -- the closer is two characters, and the content may hold - // anything at all, which is the point of the form. - int close = text.indexOf("/$", quoteAt + 2); - return close < 0 ? text.length() : close + 1; + // anything at all, which is the point of the form. Almost anything: the + // dollar escapes itself and a slash, so $$ is a dollar and $/ is a + // slash. Searching for the first "/$" substring found the slash of an + // escaped $/ instead of the closer and ended the literal early, which + // put the scanner back into code halfway through a string. + for (int i = quoteAt + 2; i < text.length(); i++) { + char c = text.charAt(i); + if (c == '$' && i + 1 < text.length() + && (text.charAt(i + 1) == '$' || text.charAt(i + 1) == '/')) { + i++; + continue; + } + if (c == '/' && i + 1 < text.length() && text.charAt(i + 1) == '$') { + return i + 1; + } + } + return text.length(); } if (quote == '/') { // Groovy's slashy literals MAY span lines, so the closing slash is looked @@ -1304,8 +1329,7 @@ && isAddCallArgument(line, i)) { || !isIdentifierChar(line.charAt(i - 1)); int after = i + configuration.length(); boolean endsToken = after < line.length() - && (line.charAt(after) == ' ' || line.charAt(after) == '(' - || line.charAt(after) == '\t'); + && (isBlank(line.charAt(after)) || line.charAt(after) == '('); if (startsToken && endsToken) { return true; } @@ -1535,9 +1559,23 @@ private static String[] statements(String text) { // against it. The parenthesis depth is already back to zero there, so // without this the closure lands in its own statement and the version // it carries is never associated with the coordinate above it. - while (i + 1 < defined.size() && opensAClosure(defined.get(i + 1))) { - i++; + // Past anything blank in between. A comment-only line leaves an empty + // statement behind it, and looking only at the very next one left the + // closure -- and the strict version inside it -- attached to nothing. + int next = i + 1; + while (next < defined.size() && defined.get(next).trim().length() == 0) { + next++; + } + while (next < defined.size() && opensAClosure(defined.get(next))) { + while (i < next) { + i++; + } statement = statement + " " + defined.get(i); + next = i + 1; + while (next < defined.size() + && defined.get(next).trim().length() == 0) { + next++; + } } int braces = trailingBraceBalance(statement); while (braces > 0 && i + 1 < defined.size()) { diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index 58f41c22512..0e408e51db2 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -563,6 +563,74 @@ public void theAlignmentCannotFailTheBuild() throws Exception { "and says so, rather than swallowing the defect"); } + /** + * Line endings are not this class's business to have an opinion about. A + * fragment written on Windows put a carriage return after + * {@code strictly}, and the token-end test accepted only a space, a tab or + * an open parenthesis -- so the call stopped being a call and the pin + * behind it was never read. + */ + @Test + public void aCarriageReturnSeparatesTokensLikeAnyOtherBlank() { + String[] endings = {"\r\n", "\n", "\r"}; + for (int i = 0; i < endings.length; i++) { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib')" + endings[i] + + " { version { strictly" + endings[i] + " '1.7.22' } }" + + endings[i]); + check("".equals(out), + "the strict call survives the line ending, got <<" + out + ">>"); + } + } + + /** + * A trailing closure still belongs to its call with a blank line between + * them. Comment stripping leaves an empty statement where a comment-only + * line was, and looking at only the very next statement left the closure + * -- and the strict version inside it -- attached to nothing. + */ + @Test + public void aTrailingClosureSurvivesABlankLine() { + String[] between = { + "\n", + "\n // why this pin is here\n", + "\n\n /* and a block one */\n\n", + }; + for (int i = 0; i < between.length; i++) { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib:1.7.22')" + + between[i] + + " { version { strictly '1.7.22' } }\n"); + check("".equals(out), + "the closure is still the call's, across <<" + + between[i].replace("\n", "\\n") + ">>, got <<" + out + ">>"); + } + } + + /** + * Inside a dollar-slashy literal the dollar escapes itself and a slash, so + * {@code $/} is a slash and not the closer. Taking the first {@code /$} + * substring ended the literal early and put the scanner back into code + * halfway through a string. + */ + @Test + public void aDollarEscapeDoesNotCloseADollarSlashyLiteral() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " def marker = $/not closed $/$ can't/$; " + + "implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22') " + + "{ version { strictly '1.7.22' } }\n"); + check("".equals(out), + "the escaped delimiter did not end the literal, got <<" + out + ">>"); + + // And $$ is a dollar, not the start of one. + String dollars = KotlinStdlibAlignment.constraintsBlock("implementation", + " def marker = $/cost $$5 can't/$; " + + "implementation('org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22') " + + "{ version { strictly '1.7.22' } }\n"); + check("".equals(dollars), + "an escaped dollar is content, got <<" + dollars + ">>"); + } + /** * Removing a comment leaves the whitespace it was. A comment separates * tokens in the language, so deleting it outright joined them: From 948257b90410570bfea8261bb67e780e68ad24ec Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 21:51:01 +0300 Subject: [PATCH 37/94] Read every spelling of a force, and expand a definition that refers to another Gradle writes a force three ways -- force the method, setForcedModules its setter, forcedModules the property -- and only the first was read. The other two hold a module just as absolutely, so an app using either kept its pre-merge base library while these constraints raised the shims to their empty jars: the JDK extension classes then live in no selected jar and the app throws on the device with nothing wrong in the build. A list of API names is a list this can have, unlike a guess at syntax. A definition may interpolate an earlier one: def v = '1.9.22' def dep = "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$v" Recorded as written, dep's version stayed the text $v -- no version, therefore below the floor, therefore the whole block suppressed for a project that was already merged-era and still had a duplicate to fix. Expanding the whole statement before recording it is the obvious fix and it is wrong: it substitutes the NAME being assigned as well, so an unreadable reassignment stops forgetting the literal it replaced. That is the same mistake pointing the other way, and it took eight seconds to find because an existing test says so. Only the value is expanded. Co-Authored-By: Claude Opus 5 (1M context){@code assigned} also accepts the property form, {@code name = ...}, + * which only Gradle's forcedModules is written as. It is not offered to + * every caller because `def strictly = false` is not a strict pin.
+ */ + private static boolean callsNamed(String statement, String call, boolean assigned) { for (int i = 0; i < statement.length(); i++) { if (isLiteralStart(statement, i)) { i = endOfStringLiteral(statement, i); @@ -966,7 +987,8 @@ private static boolean callsNamed(String statement, String call) { int after = i + call.length(); boolean endsToken = after < statement.length() && (isBlank(statement.charAt(after)) - || statement.charAt(after) == '('); + || statement.charAt(after) == '(' + || (assigned && statement.charAt(after) == '=')); if (startsToken && endsToken) { return true; } @@ -1756,13 +1778,36 @@ && isIdentifierChar(statement.charAt(nameStart))) { && isLiteralStart(statement, i)) { int end = endOfStringLiteral(statement, i); if (end < statement.length()) { - literals.put(name, statement.substring(i, end + 1)); + literals.put(name, expandedLiteral(statement, i, end, literals)); return; } } literals.remove(name); } + /** + * The literal at {@code from}, with any definitions it interpolates already + * expanded. + * + *A definition may refer to an earlier one -- + * {@code def v = '1.8.0'; def dep = "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$v"} + * -- and storing it as written made the version the text {@code $v}, which + * reads as no version and therefore as below the floor, suppressing the + * block for a project that was already merged-era.
+ * + *The VALUE only. Expanding the whole statement first was tried and + * substituted the NAME being assigned as well, so an unreadable + * reassignment stopped forgetting the literal it replaced -- the same + * mistake in the opposite direction, and the suite said so.
+ */ + private static String expandedLiteral(String text, int from, int end, + MapNote the single-line spelling of one -- {@code if (c) { dep = '...' }} -- + * never reached this: braces do not split statements, so the whole + * conditional arrives as one statement whose first token is {@code if} and + * which therefore assigns nothing. Only the multi-line form, where the + * assignment is a statement of its own, was ever applied unconditionally. + * Checked both ways before writing this.
+ */ + private static boolean opensAConditional(String statement) { + for (int k = 0; k < CONTROL_KEYWORDS.length; k++) { + if (callsNamed(statement, CONTROL_KEYWORDS[k])) { + return braceBalance(statement) > 0; + } + } + return false; + } + + /** Groovy's control structures, whose bodies are not known to run. */ + private static final String[] CONTROL_KEYWORDS = { + "if", "else", "while", "for", "switch", "try", "catch" + }; + + /** + * Records a definition, or forgets it, unless doing so under a condition + * would throw away the value that decides suppression. + */ + private static void recordDefinition(MapThe single-line spelling was never affected -- braces do not split + * statements, so {@code if (c) { dep = '...' }} arrives as one statement + * that assigns nothing -- and it is asserted here so the difference is not + * mistaken for a gap later.
+ */ + @Test + public void aConditionalReassignmentDoesNotHideAPin() { + String multiLine = KotlinStdlibAlignment.constraintsBlock("implementation", + " def dep = 'org.jetbrains.kotlin:kotlin-stdlib:1.7.22'\n" + + " if (project.hasProperty('other')) {\n" + + " dep = 'com.example:other:1.0'\n" + + " }\n" + + " implementation(dep) { version { strictly '1.7.22' } }\n"); + check("".equals(multiLine), + "the pin survives a conditional reassignment, got <<" + multiLine + ">>"); + + String oneLine = KotlinStdlibAlignment.constraintsBlock("implementation", + " def dep = 'org.jetbrains.kotlin:kotlin-stdlib:1.7.22'; " + + "if (project.hasProperty('other')) { dep = 'com.example:other:1.0' }; " + + "implementation(dep) { version { strictly '1.7.22' } }\n"); + check("".equals(oneLine), + "and the one-line form, which never assigned at all, got <<" + + oneLine + ">>"); + + // The other direction was already safe and stays that way: a conditional + // assignment TO a Kotlin coordinate is taken, because taking it suppresses. + String gained = KotlinStdlibAlignment.constraintsBlock("implementation", + " def dep = 'com.example:other:1.0'\n" + + " if (project.hasProperty('old')) {\n" + + " dep = 'org.jetbrains.kotlin:kotlin-stdlib:1.7.22'\n" + + " }\n" + + " implementation(dep) { version { strictly '1.7.22' } }\n"); + check("".equals(gained), + "a conditional assignment to a coordinate is seen, got <<" + gained + ">>"); + + // Unconditionally, a reassignment still replaces what it replaces. + String plain = KotlinStdlibAlignment.constraintsBlock("implementation", + " def dep = 'org.jetbrains.kotlin:kotlin-stdlib:1.7.22'\n" + + " dep = 'com.example:other:1.0'\n" + + " implementation(dep)\n"); + check(plain.contains("kotlin-stdlib-jdk8:1.8.0"), + "an unconditional reassignment still applies, got <<" + plain + ">>"); + } + /** * A local may be named after the DSL key it supplies. Substituting every * occurrence turned {@code group:} into a quoted string and lost the map From b083292ba42a0b11c0b664f428e49c41c46886a4 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 23:22:06 +0300 Subject: [PATCH 41/94] Count braces instead of naming the constructs that open them The list of control structures whose bodies might not run -- if, else, while, for, switch, try, catch -- was one commit old and already missing the closure: `def mutate = { dep = '...' }` runs only when something calls it, and the reassignment inside it was applied as though it had, losing an active strict pin. So the list is gone. Depth is the closed half of that question: at the top level a statement runs, and inside any open brace this cannot say. It is also costless to be wrong about, because the flag only refuses to DISCARD a coordinate -- a first definition is still recorded at any depth, which is what keeps a `def` inside dependencies { } working, and that case is asserted. The reported reproduction again did not reproduce, for the same reason as last time: braces do not split statements, so the one-line `def mutate = { dep = '...' }` arrives whole, assigns nothing, and never had the bug. Only the multi-line spelling did. Also: a release candidate of the floor is below the floor wherever it appears. As a range's inclusive ceiling it was compared numerically, so [1.7.0,1.8.0-RC2] read as reaching 1.8.0 and the constraints went in with nothing to resolve to. An inclusive ceiling now asks exactly what a plain version is asked, which already knew this. Co-Authored-By: Claude Opus 5 (1M context)Note the single-line spelling of one -- {@code if (c) { dep = '...' }} -- - * never reached this: braces do not split statements, so the whole - * conditional arrives as one statement whose first token is {@code if} and - * which therefore assigns nothing. Only the multi-line form, where the - * assignment is a statement of its own, was ever applied unconditionally. - * Checked both ways before writing this.
- */ - private static boolean opensAConditional(String statement) { - for (int k = 0; k < CONTROL_KEYWORDS.length; k++) { - if (callsNamed(statement, CONTROL_KEYWORDS[k])) { - return braceBalance(statement) > 0; - } - } - return false; - } - - /** Groovy's control structures, whose bodies are not known to run. */ - private static final String[] CONTROL_KEYWORDS = { - "if", "else", "while", "for", "switch", "try", "catch" - }; - /** * Records a definition, or forgets it, unless doing so under a condition * would throw away the value that decides suppression. diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index d61faecbf8e..7b142d5d6a5 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -563,6 +563,55 @@ public void theAlignmentCannotFailTheBuild() throws Exception { "and says so, rather than swallowing the defect"); } + /** + * An assignment inside ANY open brace is one whose execution this cannot + * establish, closures included: {@code def mutate = { dep = '...' }} runs + * only if something calls it. Named control structures were listed here + * once and the list was already missing this. + */ + @Test + public void anAssignmentInsideAClosureDoesNotHideAPin() { + String multiLine = KotlinStdlibAlignment.constraintsBlock("implementation", + " def dep = 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'\n" + + " def mutate = {\n" + + " dep = 'com.example:other:1.0'\n" + + " }\n" + + " implementation(dep) { version { strictly '1.7.22' } }\n"); + check("".equals(multiLine), + "the pin survives an uninvoked closure, got <<" + multiLine + ">>"); + + // A first definition is still recorded at any depth, which is what keeps a + // `def` inside dependencies { } working. + String nested = KotlinStdlibAlignment.constraintsBlock("implementation", + " dependencies {\n" + + " def dep = 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'\n" + + " implementation(dep) { version { strictly '1.7.22' } }\n" + + " }\n"); + check("".equals(nested), + "a definition inside a block is still read, got <<" + nested + ">>"); + } + + /** + * A release candidate of the floor is below the floor, wherever it appears. + * As a range's inclusive ceiling it was compared numerically and read as + * reaching the floor, so the constraints went in with nothing to resolve + * to. + */ + @Test + public void aPrereleaseCeilingDoesNotReachTheFloor() { + String rc = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('org.jetbrains.kotlin:kotlin-stdlib') " + + "{ version { strictly '[1.7.0,1.8.0-RC2]' } }\n"); + check("".equals(rc), + "a prerelease ceiling cannot reach the floor, got <<" + rc + ">>"); + + // The release itself can, and does. + String release = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk7:[1.7.0,1.8.0]'\n"); + check(release.contains("kotlin-stdlib-jdk8:1.8.0"), + "an inclusive release ceiling does, got <<" + release + ">>"); + } + /** * A coordinate concatenated onto a partial literal has no version here, * and unreadable is the honest answer -- reading the empty string as a From f0a5ee4ede74cc9d84b537ba2473a7577d0b8067 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 1 Sep 2026 23:46:05 +0300 Subject: [PATCH 42/94] Read a resolution rule, an empty declaration and a qualified type A rule's useVersion rewrites what was requested, silently, on the way through, so it holds the library exactly as firmly as a force and is now read as one. Such a rule names its artifact by comparing the parts -- `d.requested.group == '...' && d.requested.name == '...'` -- which is neither a coordinate nor a map entry, so a pre-merge useVersion on the base library went unseen while the shims were raised to their empty jars around it. Naming now also accepts the group and the artifact appearing as literals of their own. `def dep` with no value is still a name this knows about, and recording it is what lets a later assignment be recognised as one rather than as a write to something unrelated. It is stored with no value, which inlines as the name itself -- what an unset variable should look like. And a qualified type is ONE token: `java.lang.String dep = '...'` stopped at the first dot, read `java` as the type and `lang` as the name, and never recorded dep. Dots now belong to the token they are inside, with the extra-property case -- a dotted TARGET rather than a dotted type -- read off the token instead of the character after it. Both are asserted, because the change to one is what could break the other. Co-Authored-By: Claude Opus 5 (1M context)A resolution rule names an artifact by comparing its parts -- + * {@code d.requested.group == 'org.jetbrains.kotlin' && d.requested.name == + * 'kotlin-stdlib'} -- which is neither a coordinate nor a map entry, so + * neither of the shapes above saw it and a useVersion rewriting the base + * library went unread.
+ */ + private static boolean holdsLiteral(String line, String value) { + for (int i = 0; i < line.length(); i++) { + if (!isLiteralStart(line, i)) { + continue; + } + int end = endOfStringLiteral(line, i); + if (value.equals(stringLiteralContent(line, i)) + && !isReasonArgument(line, i)) { + return true; + } + i = end; + } + return false; } /** @@ -509,9 +534,7 @@ private static boolean holdsStrictly(String line, String artifact) { /** Whether the statement names {@code kotlin-stdlib} and not a longer name. */ private static boolean namesBaseStdlib(String line) { - return namesCoordinate(line, BASE_STDLIB) - || (declaresMapEntry(line, "group", KOTLIN_GROUP) - && declaresMapEntry(line, "name", BASE_STDLIB)); + return namesArtifactAnywhere(line, BASE_STDLIB); } /** @@ -653,9 +676,17 @@ private static String richVersionIn(String statement) { if (strict != null) { return strict; } + // A resolution rule's useVersion is as authoritative as either: it rewrites + // what was requested, silently, on the way through. + String ruled = versionInCall(statement, USE_VERSION); + if (ruled != null) { + return ruled; + } return versionInCall(statement, "require"); } + private static final String USE_VERSION = "useVersion"; + /** The quoted argument of {@code call}, found outside string literals. */ private static String versionInCall(String statement, String call) { // The same syntax-level call callsStrictly validated, not any occurrence of @@ -1023,7 +1054,8 @@ private static boolean declaresArtifactOnLine(String artifact, String configurat */ private static boolean callsForce(String statement) { // The method forms, which callsNamed now distinguishes from an assignment. - if (callsNamed(statement, "force") || callsNamed(statement, "setForcedModules")) { + if (callsNamed(statement, "force") || callsNamed(statement, "setForcedModules") + || callsNamed(statement, USE_VERSION)) { return true; } // forcedModules is only ever written as an assignment, and assigning it any @@ -1884,21 +1916,29 @@ private static void updateLiteralDefinitions(String statement, // last of them, exactly one is an assignment. int scan = i; int lastTokenStart = i; + int lastTokenEnd = i; int tokens = 0; while (scan < statement.length() && isIdentifierChar(statement.charAt(scan))) { lastTokenStart = scan; tokens++; + // A qualified name is ONE token: java.lang.String dep = '...' is a + // declaration whose type happens to have dots in it, and stopping at + // the first one read `java` as the type and `lang` as the name, so + // dep was never recorded and the pin it carried never seen. while (scan < statement.length() - && isIdentifierChar(statement.charAt(scan))) { + && (isIdentifierChar(statement.charAt(scan)) + || (statement.charAt(scan) == '.' + && scan + 1 < statement.length() + && isIdentifierChar(statement.charAt(scan + 1))))) { scan++; } + lastTokenEnd = scan; scan = skipBlanks(statement, scan); } if (tokens > 1) { declared = true; i = lastTokenStart; - } else if (tokens == 1 && scan < statement.length() - && statement.charAt(scan) == '.') { + } else if (tokens == 1) { // ext.kotlinVersion = '1.9.22' -- Gradle's extra properties, which is // how a project-wide version is nearly always written, and which // really does bind the bare name the interpolation then reads. @@ -1906,12 +1946,11 @@ && isIdentifierChar(statement.charAt(scan))) { // assignment would let `somePlugin.version = '1.0'` supply the value // for an unrelated $version and turn an unreadable version into a // confidently wrong one, which is the direction that under-suppresses. - int nameStart = skipBlanks(statement, scan + 1); - if (EXTRA_PROPERTIES.equals(statement.substring(lastTokenStart, scan).trim()) - && nameStart < statement.length() - && isIdentifierChar(statement.charAt(nameStart))) { + String only = statement.substring(lastTokenStart, lastTokenEnd); + int dot = only.lastIndexOf('.'); + if (dot > 0 && EXTRA_PROPERTIES.equals(only.substring(0, dot))) { declared = true; - i = nameStart; + i = lastTokenStart + dot + 1; } } } @@ -1929,6 +1968,15 @@ && isIdentifierChar(statement.charAt(nameStart))) { i = skipBlanks(statement, i); if (i >= statement.length() || statement.charAt(i) != '=' || (i + 1 < statement.length() && statement.charAt(i + 1) == '=')) { + if (declared) { + // `def dep` with no value yet is still a name this knows about, and + // recording it is what lets a later assignment be recognised as one. + // Without it, `def dep` then `if (legacy) { dep = '...' }` left the + // assignment looking like a write to something unrelated, so the + // coordinate it carried was never learned. A null value inlines as + // the name itself, which is what an unset variable should look like. + literals.put(name, null); + } return; } i = skipBlanks(statement, i + 1); diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index 7b142d5d6a5..790934e6153 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -563,6 +563,74 @@ public void theAlignmentCannotFailTheBuild() throws Exception { "and says so, rather than swallowing the defect"); } + /** + * A resolution rule's {@code useVersion} rewrites what was requested, + * silently, on the way through -- so it holds the library as firmly as a + * force does. Such a rule names its artifact by comparing the parts, which + * is neither a coordinate nor a map entry, and was read as naming nothing. + */ + @Test + public void aResolutionRuleHoldsWhatItRewrites() { + String rule = KotlinStdlibAlignment.constraintsBlock("implementation", + " configurations.all { resolutionStrategy.eachDependency { d -> " + + "if (d.requested.group == 'org.jetbrains.kotlin' && " + + "d.requested.name == 'kotlin-stdlib') d.useVersion '1.7.22' } }\n"); + check("".equals(rule), + "a rule pinning the base library pre-merge suppresses, got <<" + + rule + ">>"); + + // Rewriting it to a merged-era version takes nothing away. + String modern = KotlinStdlibAlignment.constraintsBlock("implementation", + " configurations.all { resolutionStrategy.eachDependency { d -> " + + "if (d.requested.group == 'org.jetbrains.kotlin' && " + + "d.requested.name == 'kotlin-stdlib') d.useVersion '1.9.22' } }\n"); + check(modern.contains("kotlin-stdlib-jdk8:1.8.0"), + "a merged-era rule keeps the alignment, got <<" + modern + ">>"); + } + + /** + * A variable declared without a value is still a name this knows, and + * recording it is what lets a later assignment be recognised as one. + */ + @Test + public void aDeclarationWithoutAValueIsStillADeclaration() { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " def dep\n" + + " if (legacy) {\n" + + " dep = 'org.jetbrains.kotlin:kotlin-stdlib:1.7.22'\n" + + " }\n" + + " implementation(dep) { version { strictly '1.7.22' } }\n"); + check("".equals(out), + "the conditional assignment is seen, got <<" + out + ">>"); + } + + /** + * A qualified type is one token. Stopping at its first dot read + * {@code java} as the type and {@code lang} as the name, so the variable + * was never recorded -- while {@code ext.kotlinVersion}, which is a dotted + * TARGET rather than a dotted type, still has to be read the other way. + */ + @Test + public void aQualifiedTypeIsOneToken() { + String[] types = {"java.lang.String", "String", "final java.lang.String"}; + for (int i = 0; i < types.length; i++) { + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " " + types[i] + + " dep = 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22'\n" + + " implementation(dep) { version { strictly '1.7.22' } }\n"); + check("".equals(out), + "a local of type " + types[i] + " is recorded, got <<" + out + ">>"); + } + + // The dotted extra property is still a property, not a type. + String ext = KotlinStdlibAlignment.constraintsBlock("implementation", + " ext.kotlinVersion = '1.9.22'\n" + + " implementation \"org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlinVersion\"\n"); + check(ext.contains("kotlin-stdlib-jdk8:1.8.0") + && !ext.contains("kotlin-stdlib-jdk7:1.8.0"), + "ext.kotlinVersion still binds its name, got <<" + ext + ">>"); + } + /** * An assignment inside ANY open brace is one whose execution this cannot * establish, closures included: {@code def mutate = { dep = '...' }} runs From 0bd89399d35b809d9bf034d3a7ff53e89a2c991e Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 2 Sep 2026 00:03:08 +0300 Subject: [PATCH 43/94] Read useTarget as a force, and a quoted map key as a key A resolution rule's useTarget replaces the whole coordinate where useVersion replaces the version, and both win silently over a constraint -- so a rule retargeting the base library to a pre-merge version left the shims raised to their empty jars around it. Not added, deliberately: dependencySubstitution's `substitute ... using ...`. It names TWO coordinates and the version scan takes the first literal, which is the one being REPLACED, so a substitution raising the library to a merged-era version would read as pinning it to the old one. Wrong in the harmless direction, but wrong, and it can go in when the scan can tell the sides apart. And a map key may be quoted. Every literal was skipped before looking for a key, so ('group': '...', 'name': '...') named no artifact at all and the strict pin inside it went unseen. Both spellings now share one reader for the value after the colon, so the delimiter rule is written once rather than twice. Co-Authored-By: Claude Opus 5 (1M context)Shared by both spellings of a key, bare and quoted, so the delimiter + * rule is read once. Stripping one character per side used to leave a + * triple-quoted group or name wearing two quotes, and both then failed + * their exact match.
+ */ + private static String valueAfterColon(String line, int colonAt) { + int at = skipBlanks(line, colonAt + 1); + if (at < line.length() && isLiteralStart(line, at) + && endOfStringLiteral(line, at) < line.length()) { + return stringLiteralContent(line, at); + } + return null; + } + /** * Whether the app strictly holds {@code kotlin-stdlib} itself below the * floor both shims depend on. @@ -1054,8 +1078,19 @@ private static boolean declaresArtifactOnLine(String artifact, String configurat */ private static boolean callsForce(String statement) { // The method forms, which callsNamed now distinguishes from an assignment. + // Gradle's ways of overriding a selected version: force and its setter, and a + // resolution rule's useVersion (a bare version) or useTarget (a whole + // coordinate). All of them win silently over a constraint. + // + // dependencySubstitution's `substitute ... using ...` is deliberately NOT + // here. It names TWO coordinates and the version scan takes the first + // literal, which is the one being replaced -- so a substitution raising the + // library to a merged-era version would read as pinning it to the old one. + // Wrong in the harmless direction, but wrong, and it can be added when the + // scan can tell the sides apart. if (callsNamed(statement, "force") || callsNamed(statement, "setForcedModules") - || callsNamed(statement, USE_VERSION)) { + || callsNamed(statement, USE_VERSION) + || callsNamed(statement, "useTarget")) { return true; } // forcedModules is only ever written as an assignment, and assigning it any diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index 790934e6153..f04ed932525 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -563,6 +563,58 @@ public void theAlignmentCannotFailTheBuild() throws Exception { "and says so, rather than swallowing the defect"); } + /** + * A map key may be quoted. Skipping every literal meant the key was never + * seen, so a declaration written that way named no artifact at all and the + * strict pin inside it went with it. + */ + @Test + public void aMapKeyMayBeQuoted() { + String[] quotes = {"'", "\"", "'''", "\"\"\""}; + for (int q = 0; q < quotes.length; q++) { + String u = quotes[q]; + String out = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation(" + u + "group" + u + ": 'org.jetbrains.kotlin', " + + u + "name" + u + ": 'kotlin-stdlib-jdk8', " + + u + "version" + u + ": '1.7.22')\n"); + check("".equals(out), + "a key quoted with " + u + " is still a key, got <<" + out + ">>"); + } + + // Mixed spellings in one declaration, which Groovy also accepts. + String mixed = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('group': 'org.jetbrains.kotlin', " + + "name: 'kotlin-stdlib-jdk8', \"version\": '1.7.22')\n"); + check("".equals(mixed), "mixed key spellings, got <<" + mixed + ">>"); + + // And a merged-era one written the same way keeps the sibling aligned. + String modern = KotlinStdlibAlignment.constraintsBlock("implementation", + " implementation('group': 'org.jetbrains.kotlin', " + + "'name': 'kotlin-stdlib-jdk7', 'version': '1.9.22')\n"); + check(modern.contains("kotlin-stdlib-jdk8:1.8.0") + && !modern.contains("kotlin-stdlib-jdk7:1.8.0"), + "the merged-era declaration is read, got <<" + modern + ">>"); + } + + /** + * {@code useTarget} replaces the whole coordinate rather than the version, + * and overrides just as absolutely as {@code useVersion} does. + */ + @Test + public void aRuleThatRetargetsIsStillARule() { + String old = KotlinStdlibAlignment.constraintsBlock("implementation", + " configurations.all { resolutionStrategy.eachDependency { d -> " + + "d.useTarget 'org.jetbrains.kotlin:kotlin-stdlib:1.7.22' } }\n"); + check("".equals(old), + "retargeting the base library pre-merge suppresses, got <<" + old + ">>"); + + String modern = KotlinStdlibAlignment.constraintsBlock("implementation", + " configurations.all { resolutionStrategy.eachDependency { d -> " + + "d.useTarget 'org.jetbrains.kotlin:kotlin-stdlib:1.9.22' } }\n"); + check(modern.contains("kotlin-stdlib-jdk8:1.8.0"), + "a merged-era retarget keeps the alignment, got <<" + modern + ">>"); + } + /** * A resolution rule's {@code useVersion} rewrites what was requested, * silently, on the way through -- so it holds the library as firmly as a From bd4b2b690d5c2320456d52dbc433806d014724b4 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 2 Sep 2026 00:56:16 +0300 Subject: [PATCH 44/94] Scan the whole generated script, and read a substitution from its replacement The dependencies block was the wrong boundary. A rule reaches the app's configurations from inside android { } just as well -- `project.configurations.all { resolutionStrategy.force '...:1.7.22' }` is accepted and executed there -- so android.gradle.androidx was app-controlled Gradle text that decided what resolves and was never read. The earlier reasoning, that the rest of the script lands where a dependency cannot be declared, was about DECLARATIONS and missed rules entirely. Enumerating the whole script rather than the block found two more of the same: android.xgradle_default_config, and the buildscript fragment that carries android.topDependency. All three are passed now, in script order, and the rule that decides needs no judgement: every getArg fragment interpolated into the script is app-supplied text, so every one of them is scanned. The judgement about which ones could matter is exactly what was wrong twice. The test moved with it -- it reads the whole gradleProps concatenation now instead of stopping at the dependencies block, which is the boundary that let this through. Verified by deleting android.gradle.androidx from the call. Also, a dependency substitution is read after all. It was declined last commit because the version scan takes the first literal and that is the side being REPLACED; the scan starts after `using` now, so it takes the replacement. In the ordinary spelling, `substitute module('g:a') using module('g:a:1.7.22')`, the replaced side carries no version at all, so the objection was narrower than it looked -- but the fix is the general one, and a substitution that RAISES the library is asserted to keep the alignment rather than suppress it. Co-Authored-By: Claude Opus 5 (1M context){@code def legacy = 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.22!!'} + * names the artifact and carries a strict marker, and on that basis alone + * it suppressed the whole block -- for a value that is never added to any + * configuration and decides nothing. A definition becomes a declaration + * when it is USED, and by then the name has been inlined and the usage is + * what this reads.
+ * + *Narrower than requiring a configuration, which was the other way to + * fix this and would have undone something deliberate: a strict pin on a + * variant configuration still shares a classpath with the one being + * constrained, so it suppresses on purpose. The distinction here is + * between a value and a call, not between one configuration and + * another.
+ */ + private static boolean isAssignedValue(String line, int quoteAt) { + int i = quoteAt - 1; + while (i >= 0 && (line.charAt(i) == ' ' || line.charAt(i) == '\t')) { + i--; + } + return i >= 0 && line.charAt(i) == '=' + && (i == 0 || line.charAt(i - 1) != '=') + && (i + 1 >= line.length() || line.charAt(i + 1) != '='); + } + /** * Whether the literal opening at {@code quoteAt} is the argument of a * reason rather than a dependency. @@ -1080,7 +1109,7 @@ private static boolean declaresArtifactOnLine(String artifact, String configurat * jar at all. Nothing fails in the build; it throws on the device. So a * forced version is read exactly like a strict one. */ - private static boolean callsForce(String statement) { + private static boolean callsForce(String statement, String artifact) { // The method forms, which callsNamed now distinguishes from an assignment. // Gradle's ways of overriding a selected version: force and its setter, a // resolution rule's useVersion (a bare version) or useTarget (a whole @@ -1094,10 +1123,20 @@ private static boolean callsForce(String statement) { // ordinary spelling, `substitute module('g:a') using module('g:a:1.7.22')`. if (callsNamed(statement, "force") || callsNamed(statement, "setForcedModules") || callsNamed(statement, USE_VERSION) - || callsNamed(statement, "useTarget") - || callsNamed(statement, "substitute")) { + || callsNamed(statement, "useTarget")) { return true; } + if (callsNamed(statement, "substitute")) { + // A substitution overrides only what it substitutes AWAY from. With the + // artifact as the TARGET -- substitute module('com.example:source') + // using module('...:kotlin-stdlib:1.7.22') -- the replacement is still + // subject to ordinary conflict resolution, so an existing 1.8.22 + // requirement raises it and nothing is pinned; reading that as absolute + // suppressed the block for a graph that had not been pinned at all. + int using = afterCall(statement, "using"); + String replaced = using < 0 ? statement : statement.substring(0, using); + return namesArtifactAnywhere(replaced, artifact); + } // forcedModules is only ever written as an assignment, and assigning it any // module list is a force. `force` as a property is the one that has to be // read: `{ force = false }` explicitly turns forcing OFF, and accepting any diff --git a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java index f2857976398..d0b90f58c52 100644 --- a/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java +++ b/maven/codenameone-maven-plugin/src/test/java/com/codename1/builders/KotlinStdlibAlignmentTest.java @@ -476,8 +476,11 @@ public void everyFragmentOfTheGeneratedBlockIsScanned() throws Exception { String builderSrc = new String(bytes, "UTF-8"); int at = builderSrc.indexOf("KotlinStdlibAlignment.constraintsBlock("); check(at >= 0, "the builder calls the alignment"); - String call = builderSrc.substring(at, builderSrc.indexOf(";", at)) - .replaceAll("//[^\n]*", ""); + // Comments go first, THEN the terminator is found: a semicolon inside the + // call's own explanatory comment truncated this slice and the test then + // reported arguments missing that were plainly there. + String fromCall = builderSrc.substring(at).replaceAll("//[^\n]*", ""); + String call = fromCall.substring(0, fromCall.indexOf(";")); // The WHOLE generated script, not just its dependencies block. The block was // the wrong boundary: android.gradle.androidx is interpolated inside @@ -501,12 +504,30 @@ public void everyFragmentOfTheGeneratedBlockIsScanned() throws Exception { while (hint.find()) { byPosition.put(Integer.valueOf(hint.start()), "getArg(\"" + hint.group(1) + "\""); } - // Only the getArg fragments are required across the whole script. Those are - // app-supplied Gradle TEXT by definition, so the rule needs no judgement - // about which of them could matter -- which is the judgement that was wrong - // twice. The script's other locals are builder-computed values (a version - // number, a namespace, a repository block); the ones inside the dependencies - // block are separately required below because they carry app text too. + // Every fragment that carries app-supplied text, by either route: a getArg + // read straight into the script, or a local that was ASSIGNED from one. + // Requiring only the direct reads missed injectRepo, which holds + // android.repositories and is interpolated into the repositories closure -- + // where a project.configurations.all { force } runs perfectly well. The + // locals are found by looking at how they are built, not by knowing their + // names, because knowing their names is what keeps being wrong. + java.util.SetA `def` inside a closure or a method is local to it, and a single flat + * map kept that value after the scope ended -- so an unrelated later + * `implementation(dep)` was read as a declaration of whatever the nested + * one held, and the constraint for that artifact was skipped as already + * satisfied. Only DECLARATIONS are taken back: a bare assignment inside a + * block updates the binding it found, which is why + * `if (legacy) { dep = '...' }` still reaches the statements after it.
+ */ + private static final class ScopedNames { + private final List