Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gradle Kotlin DSL #5122

Merged
merged 7 commits into from
Mar 7, 2025
Merged

Gradle Kotlin DSL #5122

merged 7 commits into from
Mar 7, 2025

Conversation

shanman190
Copy link
Contributor

@shanman190 shanman190 commented Mar 4, 2025

What's changed?

  • Import rewrite-kotlin (commit = 6f22b64)
  • Fix bug in package name discovery that resulted in invalid file paths on Windows [1]
  • Implement Gradle Kotlin DSL without type attribution (initially)

What's your motivation?

#3291

Anything in particular you'd like reviewers to focus on?

Nothing in particular

Anyone you would like to review specifically?

Anybody

Have you considered any alternatives or workarounds?

Could go all the way to fully type attributed, but as it stands that also involves some pretty significant interconnection with the Gradle distribution itself.

Any additional context

[1]

java.nio.file.InvalidPathException: Illegal char <
> at index 36: com/netflix/kayenta/orca/controllers

import java/lang/RuntimeException

@SuppressWarnings("ALL")
class AdminController {
  var publisher: String = ""

  @Suppress
  fun method(publisher: String) {
    this/publisher = publisher/AdminController.kt
	at java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182)
	at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153)
	at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77)
	at java.base/sun.nio.fs.WindowsPath.parse(WindowsPath.java:92)
	at java.base/sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:232)
	at java.base/java.nio.file.Path.of(Path.java:147)
	at java.base/java.nio.file.Paths.get(Paths.java:69)
	at org.openrewrite.kotlin.KotlinParser.lambda$parse$2(KotlinParser.java:148)
	at org.openrewrite.kotlin.KotlinParser$$Lambda$482/0x000002b03e1832e0.apply(Unknown Source)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:992)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
	at org.openrewrite.kotlin.KotlinParser.parse(KotlinParser.java:155)
	at org.openrewrite.kotlin.style.AutodetectTest.spinnakerTabsAndIndents(AutodetectTest.java:200)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@shanman190
Copy link
Contributor Author

The current failing tests are the same ones as the upstream rewrite-kotlin module after the recent changes to ChangeType to use the short class name rather than the fully qualified one.

@knutwannheden
Copy link
Contributor

knutwannheden commented Mar 4, 2025

Nice to see this, @shanman190 ! I will review it and test it a bit.

@knutwannheden
Copy link
Contributor

I fixed those failing tests: openrewrite/rewrite-kotlin@e0243b8

@shanman190
Copy link
Contributor Author

@knutwannheden, sounds good. I'll get that commit folded in here also.

@knutwannheden
Copy link
Contributor

If we want to keep the Git history, we could merge this first instead: #5125

@shanman190
Copy link
Contributor Author

@knutwannheden, I'm happy if we want to perform a Git merge with history of rewrite-kotlin. I'd then cherry pick my changes off of this PR and force push the updates.

@knutwannheden
Copy link
Contributor

@shanman190 I merged #5125 and rebased this PR for you.

@shanman190
Copy link
Contributor Author

Parser.Builder may need a better grouping system than just a single Class<?> entry now that the GradleParser supports both G and K types together.

@shanman190 shanman190 marked this pull request as ready for review March 7, 2025 20:52
@shanman190 shanman190 merged commit 2b1ab68 into main Mar 7, 2025
2 checks passed
@shanman190 shanman190 deleted the feature/gradle-kotlin-dsl branch March 7, 2025 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants