[JEWEL-1309] Bump detekt from v1 to v2.0.0-alpha.3#3495
[JEWEL-1309] Bump detekt from v1 to v2.0.0-alpha.3#3495DanielSouzaBertoldi wants to merge 1 commit into
Conversation
e2ca888 to
77a849c
Compare
|
btw if you checked any of the links before clicking them, let it be known that I'm disappointed in you 😈 |
77a849c to
38b4367
Compare
38b4367 to
e524bb0
Compare
|
Great work @DanielSouzaBertoldi! 👏 I'll try fixing the JPS part internally. |
rock3r
left a comment
There was a problem hiding this comment.
Great job, found a few issues to look at before we ship, but they should be quick fixes I hope
| @@ -38,8 +38,8 @@ jvm_library( | |||
| "@lib//:kotlin-stdlib", | |||
| "//libraries/kotlinx/coroutines/core", | |||
| "@lib//:jetbrains-annotations", | |||
| "@lib//:platform-jewel-detekt_plugin-jetbrains-kotlin-compiler-embeddable", | |||
| "@lib//:platform-jewel-detekt_plugin-io-gitlab-arturbosch-detekt-api", | |||
| "@lib//:platform-jewel-detekt_plugin-jetbrains-kotlin-compiler", | |||
There was a problem hiding this comment.
I don't know Bazel that well, but this seems sus: the actual dependency was migrated to kotlin-compiler here, but the detekt-plugin-binary-excludes target above still points at the old kotlin-compiler-embeddable label. lib/BUILD.bazel only seems to define platform-jewel-detekt_plugin-jetbrains-kotlin-compiler now.
Should that exclusion be updated to the new label too?
There was a problem hiding this comment.
Hmmmm maybe???? I just ran the script and kinda went with it. Since bazel builds were broken by then I couldn't reliably test if this works. I'll give a try once again and let you know
e524bb0 to
4e7c588
Compare
2b05f08 to
fa11b28
Compare
Group ID changed from io.gitlab.arturbosch.detekt to dev.detekt. Updated all library entries, SHA256 checksums, and transitive deps in both detektPlugin and detektPlugin.tests IML files.
fa11b28 to
20c26d3
Compare
rock3r
left a comment
There was a problem hiding this comment.
LGTM but leaving @nebojsa-vuksic to answer the question and/or fix things :D
Context
In order to bump CMP to 1.11.0-beta03, we have to first bump Detekt to 2.0.0-alpha.3. This PR does exactly that, fixes all migration issues and updates gradle/IML/Bazel files accordingly.
The Surprise
With this update, we cannot run tests from
detekt-pluginmodule through JPS anymore. The reason for that is because Detekt 2.0 updated its list of transitive dependencies which result in a clash of missing APIs from either IJP orkotlin-compilerdirectly 😬 Here's a more detailed explanation:Detekt v1 used to depend on kotlin-compiler-embeddable (check POM here)
v2 ditched the embeddable version for the "slim" kotlin-compiler (check POM here)
This seemingly innocent change screws up our test suite for custom detekt rules, and we start getting this exception:
Error occurred during initialization of VM
java.lang.Error: com.intellij.util.lang.UrlClassLoader.(java.lang.ClassLoader)
at java.lang.ClassLoader.initSystemClassLoader(java.base@25.0.2/ClassLoader.java:1901)
at java.lang.System.initPhase3(java.base@25.0.2/System.java:1988)
Caused by: java.lang.NoSuchMethodException: com.intellij.util.lang.UrlClassLoader.(java.lang.ClassLoader)
at java.lang.Class.getConstructor0(java.base@25.0.2/Class.java:3187)
at java.lang.Class.getDeclaredConstructor(java.base@25.0.2/Class.java:2491)
at java.lang.ClassLoader.initSystemClassLoader(java.base@25.0.2/ClassLoader.java:1888)
at java.lang.System.initPhase3(java.base@25.0.2/System.java:1988)
From what I've gathered, that's because
kotlin-compilerbundles its ownUrlClassLoaderwithout aClassLoaderconstructor. When this class ends up in the classpath, anything trying to instantiateUrlClassLoader(parent)blows up. Apparently JPS needs this specificUrlClassLoaderto start the test JVM.The only
UrlClassLoaderwith the right constructor comes from build 261's platform-loader.jar, but in this buildZipFilePool$EntryResolver.loadZipEntry(String)API got removed which detekt calls in its standalone JAR (detekt-kotlin-analysis-api-standalone-all.jarwhich bundles their own analysis API and some IntelliJ platform internals).Which means we hit a huge wall here :pain-harold:
Using
kotlin-compilermeans we crash because the expectedUrlClassLoaderconstructor can't be foundUsing
IntelliJ's ZipFilePool/UrlClassLoaderfrom platform-loader.jar we break DetektThe "good" news is that we can safely run tests via Gradle, and any other test outside the
detekt-pluginmodule can be run normally via JPS