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

Android Studio / IDEA plugin yields different results than running ktfmt #444

Open
gino-m opened this issue Mar 24, 2024 · 11 comments
Open
Labels
question Further information is requested

Comments

@gino-m
Copy link

gino-m commented Mar 24, 2024

Originally posted in cortinico/ktfmt-gradle#264

🐛 Describe the bug

Formatting with the Android Studio ktfmt plugin produces a different result than ktfmtFormat via Gradle and https://facebook.github.io/ktfmt/.

⚠️ Current behavior

./gradlew ktfmtFormat target (or trigger via IDE) provided by Gradle plugin com.ncorti.ktfmt.gradle version 0.17.0:

@Retention(AnnotationRetention.RUNTIME) annotation class SharedViewModel

"Format code" using Android Studio ktfmt plugin 1.1.0.47:

@Retention(AnnotationRetention.RUNTIME)
annotation class SharedViewModel

More deltas can be found in https://github.com/google/ground-android/pull/2412/files. Base files are formatted via the Gradle plugin, modified files with AS plugin.

✅ Expected behavior

Both formatters produce the same result.

@cortinico @cgrushko @sufyanAbbasi @shobhitagarwal1612 FYI

@hick209
Copy link
Contributor

hick209 commented Mar 25, 2024

@greyhairredbear do you know if this is related to #420?

@hick209
Copy link
Contributor

hick209 commented Mar 26, 2024

I tried to format you code here and it fails due to the @interface there, as that's not valid Kotlin.

Could you revisit and share a piece of code where we could repro the issue?

@hick209 hick209 added the question Further information is requested label Mar 26, 2024
@greyhairredbear
Copy link
Contributor

greyhairredbear commented Mar 27, 2024

@greyhairredbear do you know if this is related to #420?

Unfortunately, not really. My original thought was that this might be because of different ktfmt versions, but according to the original post in the gradle plugin, this doesn't seem to be the case.

I can try if #420 produces the expected results.

UPDATE: @hick209 of course, you are correct, the @interface is not valid Kotlin... @gino-m are you sure this is a .kt file you are referencing here?

PS: @gino-m I suppose it would be helpful to also include the versions used in this post.

@greyhairredbear
Copy link
Contributor

Just FYI: When using #420, the code stays unchanged (regardless of the formatting) due to it not being valid Kotlin. After removing the @interface-portion, the output is the following:

/**
 * Annotates view models to indicate that a single instance should be shared by all fragments in an
 * activity. Only one instance of such view models will be created per activity by {@link
 * ViewModelFactory#get(Fragment, Class)}.
 */
@Retention(RetentionPolicy.RUNTIME) public SharedViewModel {}

@gino-m
Copy link
Author

gino-m commented Mar 27, 2024

@hick209 So sorry, was moving too fast. That was Java - added a Kotlin example .

@greyhairredbear The formatting in the above snippet was produced by Android Studio ktfmt plugin 1.1.0.47, the Gradle plugin com.ncorti.ktfmt.gradle version 0.17.0. Added these to the updated issue description and shared additional deltas in https://github.com/google/ground-android/pull/2412/files.

@blipinsk
Copy link

@gino-m this might be related: cortinico/ktfmt-gradle#277

@greyhairredbear
Copy link
Contributor

FYI: Just tried the new version of the plugin released last week and the code you mentioned

@Retention(AnnotationRetention.RUNTIME)
annotation class SharedViewModel

gets reformatted to the code produced by the gradle plugin

@Retention(AnnotationRetention.RUNTIME) annotation class SharedViewModel

Seems to me this is not an issue anymore - probably has been fixed with #420. @gino-m could you verify that also is the case with the code you've mentioned, so we can close this?

@gino-m
Copy link
Author

gino-m commented Apr 30, 2024

Confirmed, I upgraded to 1.1.0.49 and the outputs for this particular use case now match.

There are still some differences, which I've tried to capture here: https://github.com/google/ground-android/pull/2456/commits

The first run of the latest AS plugin across our codebase introduced many changes, most of which are not modified by running ktfmt via Gradle again. Subsequent commits demonstrate formatting which is changes by both entry points on each run, namely: google/ground-android@b9736a1

@blipinsk
Copy link

@gino-m did you also update the ktfmt-gradle to the newer version (0.18.0)? I suspect the intellij plugin is working correctly but your codebase was formatted incorrectly with the older Gradle plugin

@gino-m
Copy link
Author

gino-m commented Apr 30, 2024

Good catch. I upgraded ktfmt-gradle to 0.18.0 and reformatted (google/ground-android@9dc9da5).

Reformatting with AS ktfmt 1.1.0.49 still yields several diffs (google/ground-android@e6a4974), but subsequent runs of ktfmt-gradle only modify three files (google/ground-android@ea60727). Two of these changes get undone when running the AS plugin again (google/ground-android@ebe230e).

At this point I would expect the diffs, if any, to be symmetrical, but running ktfmt-gradle again only modifies one file (google/ground-android@81371be). After that, subsequent runs fight over a single delta (google/ground-android@e26f8d9).

Edit: Pasting screenshot here for convenience. Before = ktfmt-gradle 0.180, After = AS ktfmt 1.1.0.49:

image

If that delta is resolved the formatting should be stable across both plugins after each is run several times.

@greyhairredbear
Copy link
Contributor

What is the output of the CLI? I ran ktfmt --google-style Config.kt locally and got the second output - so it seems the plugin is behaving as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants