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

Alphabetical import ordering enabled by default is a problem in Android Studio/IntelliJ #527

Closed
dimsuz opened this issue Jul 15, 2019 · 52 comments · Fixed by #749
Closed
Milestone

Comments

@dimsuz
Copy link

dimsuz commented Jul 15, 2019

In our project ktlint 0.34.0 now reports a lot of incorrect import ordering, although we use a default import layou of IDE. All our developers have "Optimize imports" enabled by default, but this still creates an order like the following (which seems to be default one of Kotlin IDE plugin):

import ru.example.a
import ru.example.b
import ru.example.c
import javax.inject.Inject

Notice that the last one comes in non alphabetical order.
The unfortunate thing is that this cannot be changed.

Import order editing is there only for Java CodeStyle, kotlin plugin seems not to provide it. Correct me if I'm wrong.

If you think that this is still something that should be default, then please close.

But currently I suspect that many Android Studio/IntelliJ users will be disabling this rule, otherwise they'll have an endless conflict between IDE autoformatter/ktlint.

@shashachu
Copy link
Contributor

@dimsuz as of 0.34.0 you can disable rules via the .editorconfig: https://github.com/pinterest/ktlint#custom-editorconfig-properties

So in this case it would be disabled_rules=import-ordering

@dimsuz
Copy link
Author

dimsuz commented Jul 15, 2019

Thanks for the hint, will do that! Just wanted to report this in case having it as a default is up for review.

@shashachu
Copy link
Contributor

Thanks for the report! Of course anything and everything is up for discussion; I will close this ticket for now but I will keep an eye out for feedback if this rule proves controversial.

@Tapchicoma
Copy link
Collaborator

You also can follow this issue from Kotlin side: https://youtrack.jetbrains.com/issue/KT-10974

@dimsuz
Copy link
Author

dimsuz commented Jul 16, 2019

Thanks, voted!

@jrehwaldt
Copy link

Adding a property to .editorconfig -- which itself is flagged by yet other linters as invalid -- in order to suppress this lint issue is rather funny. I believe this ticket should be reopened. As it stands, any project with at least a single IntelliJ user (for OpenSource projects I'd argue actually all projects are impacted) has to disable this rule.

@shashachu
Copy link
Contributor

Hi Jan,

Just to get more information, can you give an example of where the IntelliJ optimized imports ends up not in Alphabetical order? I understand it's frustrating to have ktlint disagree with IDEA, it's not clear to me whether the IDEA ordering is a bug or desired behavior. Thanks!

@shashachu shashachu reopened this Jul 23, 2019
@0legg
Copy link
Contributor

0legg commented Jul 23, 2019

fyi: IDEA puts java/javax imports at the bottom, contrary to ktlint expectations.

@dimsuz
Copy link
Author

dimsuz commented Jul 23, 2019

Yep, I have provided an example of invalid order in the original post, that is where it did go wrong for us.

@shashachu
Copy link
Contributor

Thanks. I just added a comment onto https://youtrack.jetbrains.com/issue/KT-10974 to ask if putting all the java and javax imports at the bottom is a bug or desired behavior. There are a number of instances inside of ktlint rules where we do things to explicitly match IntelliJ's formatting, but if this is a bug, I'm not sure I want to build the same bug into ktlint.

@bhoogter
Copy link

bhoogter commented Jul 25, 2019

It does invalidate the claim on the ktlint README that...

...it makes Intellij IDEA's built-in formatter produce 100% ktlint-compatible code.

If this is 'expected behavior', calling the cli suggested ktlint --apply-to-idea-project becomes mostly useless since a 98% match still means going to the terminal to do ktlint -F, as no one is going alphabetize imports manually (as opposed to other non-automatically fixed issues which can be fixed by user intervention with moderate effort).

@shashachu
Copy link
Contributor

It does invalidate the claim on the ktlint README that...

...it makes Intellij IDEA's built-in formatter produce 100% ktlint-compatible code.

If this is 'expected behavior', calling the cli suggested ktlint --apply-to-idea-project becomes mostly useless since a 98% match still means going to the terminal to do ktlint -F, as no one is going alphabetize imports manually (as opposed to other non-automatically fixed issues which can be fixed by user intervention with moderate effort).

Sure. I understand your frustration, I just want to make sure the IDEA Optimize Imports ordering is not going to change before mimicing the same behavior in ktlint.

@gabrielittner
Copy link
Contributor

Just want to add that if the rule is changed it shouldn't be changed for --android:

Import statements for classes, functions, and properties are grouped together in a single list and ASCII sorted.

https://developer.android.com/kotlin/style-guide#structure

@mkobit
Copy link

mkobit commented Jul 26, 2019

For what it is worth, the Google Java style guide used to have the java, javax, and some others separate from library imports. Now (I don't remember when it changed), they just have them grouped by non-static and static. From https://google.github.io/styleguide/javaguide.html#s3.3.3-import-ordering-and-spacing:

Imports are ordered as follows:

  1. All static imports in a single block.
  2. All non-static imports in a single block.

If there are both static and non-static imports, a single blank line separates the two blocks. There are no other blank lines between import statements.

Within each block the imported names appear in ASCII sort order. (Note: this is not the same as the import statements being in ASCII sort order, since '.' sorts before ';'.)

Which the Google Checkstyle stylesheet also uses.

I don't see import order mentioned on https://kotlinlang.org/docs/reference/coding-conventions.html

@JakeWharton
Copy link
Contributor

And Google's Kotlin style guide (not yet public) also matches Java and the Android Kotlin guide in that regard.

@shashachu
Copy link
Contributor

Just want to add that if the rule is changed it shouldn't be changed for --android:

Import statements for classes, functions, and properties are grouped together in a single list and ASCII sorted.

https://developer.android.com/kotlin/style-guide#structure

One thing we're also considering is breaking the android-specific rules into a different ruleset rather than using a flag.

@johntowell
Copy link

It does invalidate the claim on the ktlint README that...

...it makes Intellij IDEA's built-in formatter produce 100% ktlint-compatible code.

If this is 'expected behavior', calling the cli suggested ktlint --apply-to-idea-project becomes mostly useless since a 98% match still means going to the terminal to do ktlint -F, as no one is going alphabetize imports manually (as opposed to other non-automatically fixed issues which can be fixed by user intervention with moderate effort).

Sure. I understand your frustration, I just want to make sure the IDEA Optimize Imports ordering is not going to change before mimicing the same behavior in ktlint.

That bug was open in 2016 it looks like, not sure it is going to be resolved quickly. This issue caused quite a bit of headache for us today. Is there any reason not to keep inline with Intellij until if/when it changes?

@VaclavDedik
Copy link

Our whole backend team has this problem, so it would really be great if we could turn off this inspection by ktlint by some configuration option (at the very least) :)

@Tapchicoma
Copy link
Collaborator

@VaclavDedik check this comment to the way how to disable it: #527 (comment)

@greenkiwi
Copy link

We definitely have this issue too.

How painful would it be to add a configuration option for ktlint to handle this case? Even if we have to remove it later once IntelliJ sorts this out.

@dimsuz
Copy link
Author

dimsuz commented Sep 18, 2019

@greenkiwi There's a way to disable this check, see the first comment above.

@greenkiwi
Copy link

@dimsuz - We've disabled the check. It would be great to have the option to have it match the IntelliJ order. i.e. still check, just with java/javax at the end.

@gigaSproule
Copy link

Any further changes on this? Seems like the kotlin.* imports are being put at the bottom as well.

@liutikas
Copy link
Contributor

liutikas commented Oct 2, 2019

The following code block will cause ktlint to report incorrect order

import kotlinx.coroutines.CoroutineDispatcher
import kotlin.coroutines.CoroutineContext

object DirectDispatcher : CoroutineDispatcher() {
    override fun dispatch(context: CoroutineContext, block: Runnable) {
        block.run()
    }
}

This seems to be the default IJ import ordering for koltin and it mismatches ktlint.

mcauto pushed a commit to mcauto/todolist-android that referenced this issue Jun 12, 2020
mcauto pushed a commit to mcauto/todolist-android that referenced this issue Jun 13, 2020
* feat(repository): apply repository modularization with dagger2

* feat(app): use repository module with coroutine

* fix(ktlint): ignore import ordering

pinterest/ktlint#527

* docs: modify readme

* feat(ui): add username, password

* fix: repository test, jdk11(x64), gradle jvm heap size up
vadeg added a commit to zalando/zally that referenced this issue Jun 19, 2020
* Update `gradle` to `6.5`
* Update `klint` version and add `klint-idea` plugin to generate IDEA specific tasks
* Disable `import-ordering` klint check (see [PR](pinterest/ktlint#527) for details.
vadeg added a commit to zalando/zally that referenced this issue Jun 19, 2020
* Update `gradle` to `6.5`
* Update `klint` version and add `klint-idea` plugin to generate IDEA specific tasks
* Disable `import-ordering` klint check (see [PR](pinterest/ktlint#527) for details.

Fixes #1155
peterhaochen47 pushed a commit to cloudfoundry/credhub that referenced this issue Jun 30, 2020
* Upgrade gradle ktlint plugin
* The old version has a [bug](pinterest/ktlint#527) where it cannot properly sync formatting
rules with intellij
* Auto-format code accordingly by running `./scripts/lint.sh`

Co-authored-by: Peter Chen <pchen@pivotal.io>
sondr3 added a commit to echo-karriere/backend that referenced this issue Aug 2, 2020
friendlyanon pushed a commit to friendlyanon/gateway2020-controller that referenced this issue Aug 22, 2020
@camsteffen
Copy link

For those watching at home, this is also fixed on the IDEA side in Kotlin plugin 1.4 - JetBrains/kotlin#3336

Archrahkshi added a commit to Archrahkshi/spotifine that referenced this issue Dec 1, 2020
Lists

ktlint problem: pinterest/ktlint#527
ktlint import ordering ignored, using Android Studio default settings
Archrahkshi added a commit to Archrahkshi/spotifine that referenced this issue Dec 3, 2020
Massive changes

ktlint problem: pinterest/ktlint#527
ktlint import ordering ignored, using Android Studio default settings
@fishy
Copy link

fishy commented Jan 9, 2021

I guess this is related, but if it's not, I can create a new issue for it.

When using ktlint 0.40.0, it tries to move my kotlinx.* imports to above both java.* and kotlin.* imports, which looks really weird to me. This is the diff it generated by running ktlint -F:

@@ -15,14 +15,14 @@ import android.service.notification.StatusBarNotification
 import androidx.core.app.NotificationCompat
 import androidx.core.app.NotificationManagerCompat
 import androidx.core.content.edit
-import java.security.SecureRandom
-import java.util.concurrent.locks.ReentrantLock
-import kotlin.concurrent.withLock
-import kotlin.text.Regex
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
+import java.security.SecureRandom
+import java.util.concurrent.locks.ReentrantLock
+import kotlin.concurrent.withLock
+import kotlin.text.Regex

I'm not sure if this is intentional (to match intellij behavior, for example), or a bug.

@romtsn
Copy link
Collaborator

romtsn commented Jan 9, 2021

@fishy Yes, this is intentional as we are trying to match intellij default behavior, where java, javax and kotlin imports are coming after all others.

@fishy
Copy link

fishy commented Jan 9, 2021

@fishy Yes, this is intentional as we are trying to match intellij default behavior, where java, javax and kotlin imports are coming after all others.

Thanks! But maybe kotlinx.* should also be part of that exception list to be put at the bottom? (consider we also have javax.* in the group)

@fishy
Copy link

fishy commented Jan 9, 2021

oh never mind just saw that intellij also doesn't add kotlinx there.

@ajans
Copy link

ajans commented Sep 8, 2021

I just found out with the "Import Layout" settings in IDEA/AS, you can simply enforce lexicographical import-ordering by removing all entries from "Import Layout" settings for Kotlin aside from "import all other imports" and disable the option "Import aliases separately".

Then IDEA/AS orders imports just the same as ktlint does by default with --android flag (tested with the unofficial ktlint-plugin in AS, which includes ktlint 0.42.1 atm according to changelog).

@svenjacobs
Copy link

Although my settings look like this in Preferences / Editor / Code Style / Kotlin / Imports, Android Studio still puts aliased imports first which is a flagged by the import-ordering rule.

Screenshot 2022-09-16 at 12 05 11

I'm unable to get AS to organize imports the way ktlint expects it.

Android Studio Dolphin 2021.3.1
ktlint 0.47.1

@igorwojda
Copy link

Ktlint 1.0.1

file .editorconfig

[*.{kt,kts}]
ktlint_standard_import-ordering = disabled

@azaka01
Copy link

azaka01 commented Jun 3, 2024

ktlint_standard_import-ordering = disabled

thank you @igorwojda

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

Successfully merging a pull request may close this issue.