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

//noinspection in imports throws a lexicographical error #2104

Open
dmytroKarataiev opened this issue Jun 30, 2023 · 5 comments
Open

//noinspection in imports throws a lexicographical error #2104

dmytroKarataiev opened this issue Jun 30, 2023 · 5 comments

Comments

@dmytroKarataiev
Copy link

Expected Behavior

ktlint should support //noinspection and not fail the import ordering when a suppression on an import line is present: https://pinterest.github.io/ktlint/0.50.0/rules/standard/#import-ordering

Observed Behavior

import com.app.android.Direct
//noinspection NetworkModelSuspiciousImport
import com.app.android.network.Eligibility

This fails with the following ktlint error:
Imports must be ordered in lexicographic order without any empty lines in-between with "java", "javax", "kotlin" and aliases in the end -- no autocorrection due to comments in the import list (import-ordering)

Steps to Reproduce

Add //noinspection comment to suppress a lint check in the list of imports.

Your Environment

  • Version of ktlint used: 0.46.1
@segunfamisa
Copy link

Hey folks, I'd like to give this a go, if it's still up for grabs :-)

@paul-dingemans
Copy link
Collaborator

Go ahead, if you want.

@segunfamisa
Copy link

segunfamisa commented Aug 27, 2023

Hey folks,

Could you clarify the expected behaviour of this issue, please?

  1. Is it expected that only the import declarations without //noinspection are considered? i.e., should the imports with the //noinspection be ignored in the sorting order?
  2. Is it expected for ktlint to auto-correct the sorting? If yes, what is the expected reordering for this piece of code below?
  3. If no, and we just want an error, which line is the error supposed to point to? My gut is that its the first wrongly placed import statement?
import android.view.View
 //noinspection intentional ordering
import android.app.Activity
import android.view.ViewGroup
import android.app.FragmentActivity

@paul-dingemans
Copy link
Collaborator

//noinspection intentional ordering

Your example makes the issue more complex assuming that intential ordering is an inspection which can be suppressed. I think that that original example just requires that a //noinspection comment in the import list does not throw an exception. Your example seems to intend that the ordering of the list itself should not be changed.

I agree that the //noinspection comment should not throw an exception and that the comment sticks to the next import when reordering the imports. I am not sure whether ktlint should support //noinspection intentional ordering unless it is really required to compile code.

  • Is it expected that only the import declarations without //noinspection are considered? i.e., should the imports with the //noinspection be ignored in the sorting order?

Please reverse engineer the behavior in IntelliJ IDEA default formatting.

  • Is it expected for ktlint to auto-correct the sorting? If yes, what is the expected reordering for this piece of code below?

If deterministically possible then autocorrect is required

  • If no, and we just want an error, which line is the error supposed to point to? My gut is that its the first wrongly placed import statement?

Yes, that seems the correct approach.

@paul-dingemans
Copy link
Collaborator

//noinspection intentional ordering

Your example makes the issue more complex assuming that intential ordering is an inspection which can be suppressed. I think that that original example just requires that a //noinspection comment in the import list does not throw an exception. Your example seems to intend that the ordering of the list itself should not be changed.

No Kotlin inspection is found which is related to import order.

So for now, it is sufficient to keep a //noinspection comment to be attached to the import on the next line. According to https://www.jetbrains.com/help/idea/disabling-and-enabling-inspections.html#suppress-inspections:

For example in Java, if you suppress an inspection for a class, a method, or a field, the IDE adds the @SuppressWarnings annotation. For statements, the //noinspection comment is added.

For now, it seems safe to assume that same applies to Kotlin.

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

No branches or pull requests

3 participants