Please forgive my bikeshedding, but I beg to differ with the "no wildcards" rule. My primary argument is that intellij does add wildcard imports in the default configuration, and I think I should not have to configure intellij to play nice with ktlint. Adding // ktlint-disable no-wildcard-imports is a very ugly workaround.
Another acceptable solution for me would be if the format action actually resolved wildcard imports into individual imports, which it does not as of version 0.7.0. I don't want to argue about what's better, I just want to have a consistent style without requiring manual intervention.
fwilhe
All 57 comments
Hi @fwilhe.
I presume you have read https://github.com/shyiko/ktlint#-with-intellij-idea (option 1).
Resolving * imports into individual imports is actually on my todo list. The thing is, it will require deeper integration with the compiler chain and it's hard to say how much time it's going to take. But, you (and anyone else bold enough to venture that way) are welcome to try it out. I'd happy to merge such PR in.
tldr; One way or another it's going to be done.
shyiko
on 30 May 2017
tldr; One way or another it's going to be done.
Awesome, thanks for the prompt reply.
fwilhe
on 30 May 2017
Would it be possible to override this rule via the .editorconfig file? Seems like adding no-wildcard-imports = false would be a pretty elegant solution.
asarazan
on 27 Jun 2017
@aem Thank you for the PR. It looks ready to merge (as soon as we have reached some kind of consensus here).
@asarazan My concern is - EditorConfig isn't really supposed to deal with language-specific settings. I'm aware of https://github.com/editorconfig/editorconfig/wiki/editorconfig-properties but I'm not sure how I feel about it. On the one side adding this property feels like abusing .editoconfig and bike-shedding, on the other - it could reduce contention.
BTW: To my knowledge, Intellij does not support anything but
indent_size,continuation_indent_size,tab_width,indent_styleat the moment (note that all of them are editor-centric).
shyiko
on 29 Jun 2017
@shyiko I definitely agree that this feels a bit like fitting a square editorconfig into a round hole, not sure where else it would go though. i just went with editorconfig since it's what was used for the indents (which is admittedly the right use of editorconfig)
from our perspective, due to some very specific needs of our product we have enums with 15+ cases, importing each of those without wildcards introduces a lot of extra loc and makes diffs much longer/harder to read. we'd love an override here, regardless of what form it takes
aem
on 29 Jun 2017
+1
raffek
on 13 Jul 2017
I just looked at a few random sample projects in the official Kotlin group (https://github.com/Kotlin), and I saw no source file that did not use wildcard imports (ok, my sample size was small, but I don't think I just got the few exceptions). I don't think that there is consensus for this rule among the Kotlin devs, so I'd argue for just drop it.
fwilhe
on 18 Jul 2017
@fwilhe i'd argue the exact opposite. some people prefer no wildcards, but the default IntelliJ config automatically adds them. forcing all consumers of ktlint who use the default IntelliJ config to change their IntelliJ config and then go through potentially 100s of Kotlin files to expand all of the imports seems to be bikeshedding at its best, so the alternative is to allow a 1-line config or single command-line flag override to allow us to get back to writing code ASAP
aem
on 18 Jul 2017
@aem I'm not sure if there is a misunderstanding. I don't really get what your position is.
The first feature that the readme of ktlint lists is "no configuration". And I value this feature. I think the one thing that Go really got right is gofmt. I value "no configuration" more then "consistency of import style". To be honest, I mostly don't see the imports because they are collapsed and managed by the IDE.
fwilhe
on 18 Jul 2017
@fwilhe check the explanation in #43 when allowing 2 spaces. there's a precedent for value being assigned to this tool being a solution that doesn't require reformatting your whole codebase, even if it requires an extra 1 line of config. i don't see this as being too different from that
aem
on 18 Jul 2017
I'm aware of #43, but I don't think this justifies more configuration. Feels a bit like a "broken window" case. "We got one option, sure a second option won't hurt". What is so bad about not checking if there are wildcard imports?
fwilhe
on 18 Jul 2017
so if EditorConfig isn't really supposed to deal with language-specific settings, is it possible to have some configuration files, e.g. ktlint.config, which specifies the lint rules and the library read the configuration from it?
jackylimel
on 30 Aug 2017
+1 for @jackylimel 's suggestion of a ktlint.confg
ghost
on 12 Sep 2017
Yeah besides ktlint being unable to resolve the imports out (expand them) this choice does not make sense to me. Kotlin allows top level functions, which allows us to create test factories that only need to be imported as one wildcard import. If we expanded every single import our test files would triple in size just from import statements.
snowe2010
on 21 Sep 2017
I totally agree with you @snowe2010
jackylimel
on 21 Sep 2017
As a datapoint, the source for kotlin contains over 6000 wildcard imports across 35303 kotlin source files. Personally I embrace wildcard imports because they increase readability.
BenjaminSchaaf
on 26 Oct 2017
Any progress?
henrik242
on 20 Dec 2017
Just to drop my 2 cents here:
I don't get why wildcard imports are considered to be evil. If they were, Kotlin would not copy them from Java - they seem to have dropped a lot of other baggage.
Also some inconsistencies:
- The kotlin coding conventions don't even mention imports.
- Jetbrains' yole opinion is that imports are for the IDE to manage
- The only place where import guidelines exist is the google style guide
Since this linter is based on both style guides, there is a conflict here and I'd personally err towards abandoning that check. and leaving it for the IDE to manage.
Rosomack
on 8 Jan 2018
Although I can understand that wildcard imports can be a source of problems, in my experience programming with Scala, Java and Kotlin, I have actually never seen any compilation problem coming from these imports.
Also, IntelliJ IDEA has become a standard in our industry and it has a pretty decent default configuration. I am using Ktlint to make sure that developers don't push code that hasn't been formatted to this standard, not to apply a different set of rules.
So, my take is that Ktlint should - by default or through some configuration - apply the rules that come by default with IntelliJ IDEA, and it seems that most people who contributed to this thread came to the same conclusion.
I think the idea of a ktlint.config file (or whatever its name is) is great, and since we're all developers, we could all contribute to this addition to Ktlint.
Obviously, this is not line with your original idea of the tool, @shyiko, so I guess the key question is: if we made a PR to allow configuring the rules, would you be open to accepting this PR?
aseigneurin
on 24 Jan 2018
@aseigneurin At the point at which that idea is accepted, I would suggest creating a fork of this project and publishing it under a different name. While I am in favor of wildcard imports, a config file seems to be antithetical to this project (aiming to be anti-bikeshedding) and thus a PR to do that would be better merged into a separate fork rather than this project. I'm all in favor of my PR being merged, but I don't think it makes sense to merge it or shoehorn it into the project just because it's there. If it's decided that it doesn't fit into this project then there's no need to force it.
aem
on 24 Jan 2018
I'd actually suggest disabling this rule by default unless you can match the #imports before Android Studio automatically uses a wildcard.
This is one of the pain points when I first started using ktlint
kenyee
on 5 Apr 2018
So this is a rule that isn't mentioned in the guidelines that the linter is said to implement; and the only argument against allowing a config flag for disabling said rule is that someone added the term anti-bikeshedding in the readme?
Seems like the readme should be altered a bit: "capture (reflect) official code style from kotlinlang.org and Android Kotlin Style Guide and my own personal shoot-from-the-hip formatting preferences."
Sorry, my bad, I based my conclusion on other comments; The guide lines does specify explicitly that wild card imports are to be avoided.
paramsen
on 12 Apr 2018
@paramsen
From the official Android Kotlin Style guide, Import statements section:
"Wildcard imports (of any type) are not allowed."
shyiko
on 12 Apr 2018
@shyiko I am interested in this issue because like others I think the linter should work with the default import behavior of the IDE. I don't really want to make my whole development team go through the IDE steps to make it work with the linter. However, I also agree the linter should follow the official style guide...
I made an issue in the Android Kotlin Style Guide repo to change this rule if anyone would like to give it support. If the guide is updated, then we can update the linter and everything can work in unison.
JacobMuchow
on 13 Apr 2018
Apparently they really don't care that this is a blocker for many people (as indicated by how long this issue has been open for along with the "what are your blockers to start using ktlint"), maybe someone should just create a fork and remove the rule altogether then publish it for public use.
Zhuinden
on 3 Dec 2018
@Zhuinden that's the beauty of open-source. each maintainer can choose how to progress with their own projects, and if you disagree with the direction of that project you're more than welcome to create your own that you feel better represents its best interests. we don't all have to agree on every decision in every project
aem
on 3 Dec 2018
I think most people switched to using https://github.com/arturbosch/detekt due to issues like this.
snowe2010
on 3 Dec 2018
Won't hurt to mention what JetBrains says https://ktor.io/quickstart/code-style.html
saschpe
on 8 Mar 2019
I think we just need to create a new @Option in this file which would take a comma separated list of ruleset IDs which would be filtered and therefore skipped here and here and that's it.
Just add no-wildcard-imports as an argument to ktlint for that new option, and it wouldn't be run.
class NoWildcardImportsRule : Rule("no-wildcard-imports") {
You might want to add some unit tests, because that's a thing.
Who's doing the PR?
Zhuinden
on 12 Apr 2019
woah, pinterest is in charge now. Anyway Intellij has an actual style guide now and it includes wildcard imports. ktlint should match that, at least as a default.
snowe2010
on 12 Apr 2019
@Zhuinden You are! :)
henrik242
on 16 Apr 2019
Using kotlin-android-extensions (for the synthetic view accessors) without wildcard imports is just pain.
strooooke
on 9 May 2019
The fact that this is still an argued point is ridiculous. It's now JetBrain's standard to allow wildcards, this plugin should match by default. Add an option to disallow, but default to allowing. Wildcards aren't great in Java, but they're fantastic in Kotlin.
snowe2010
on 22 May 2019
They're literally the same thing in both languages.
JakeWharton
on 22 May 2019
@snowe2010 is the JetBrain standard you're referencing this one? https://ktor.io/quickstart/code-style.html
shashachu
on 22 May 2019
My general opinion is that if this is actually preventing widespread adoption of this tool, the rule should be configurable.
In a pre-1.0 tool, I'm okay with short-term, non-ideal solutions, with the understanding that they may get pulled/broken at any time. The long-term solution will likely be something akin to a .ktlintignore or .ktlintconfig file where we can disable rules on a per-path basis.
In the short term, I see a few pretty simple solutions:
1) Pull Wildcard imports rule from standard ruleset - projects wanting that rule can bring it in as a custom rule
2) Resurrect https://github.com/pinterest/ktlint/pull/61, acknowledging that it's kind of abusing .editorconfig.
3) Add it to command line options, per this comment https://github.com/pinterest/ktlint/issues/48#issuecomment-482400103
shashachu
on 22 May 2019
@JakeWharton they're really not. Java has no concept of top-level functions, that by itself is a fantastic reason to always allow them in Kotlin, but not in Java.
@shashachu I'm referring to the Editor > Code Style > Kotlin > Set From > Predefined Style > Kotlin Style Guide setting in IntelliJ. I do not know how well this matches up with the ktor guide, though I know they're supposed to be the same thing.
snowe2010
on 22 May 2019
Java has static imports of functions which behave the exact same way. So no.
JakeWharton
on 22 May 2019
And static imports are not used in nearly the wide variety of circumstances that top-level functions are. We converted over 500k lines of Java code straight to kotlin. Switching from wildcard imports to non-wildcards ballooned each file by about a hundred lines, depending on the context, due to the number of uses of top-level functions. They're not the same thing, no matter how much they act like it.
snowe2010
on 22 May 2019
Good thing source file lines are free, import lists are collapsed automatically, and the list is managed automatically by the IDE. What are you optimizing for?
JakeWharton
on 22 May 2019
If you don't care about it then why are you here arguing. I do care, and my team cares. This tool isn't following _the standard set by the makers of the language_, and I think it should be changed. In response to your questions:
- source lines aren't free, every additional line is another line for Sonar to manage and more for us to pay for, especially at the rate of 100 lines per file over tens of thousands of files
- import lists still need to be read through at some point, the shorter the better.
- IDEs aren't perfect, and IntelliJ manages to mess up non-wildcard imports all the time.
snowe2010
on 22 May 2019
For the record, I agree with the rule (I don't like wildcard imports), but from a ktlint adoption standpoint, making it optional is the right thing to do. It seems like this is a hard blocker for many projects.
shashachu
on 22 May 2019
I agree that these things should be configurable. One of my favorite libraries is rubocop and every single thing is configurable. I believe ktlint should be the same, if the maintainers want a wide adoption.
snowe2010
on 22 May 2019
source lines aren't free, every additional line is another line for Sonar to manage and more for us to pay for, especially at the rate of 100 lines per file over tens of thousands of files
LOL what a dumb tool
import lists still need to be read through at some point, the shorter the better.
What information are you intending to glean from reading a wildcard import that is better served than individual imports?
IDEs aren't perfect, and IntelliJ manages to mess up non-wildcard imports all the time.
IDE bugs are orthogonal and can be simply be filed and fixed. You aren't changing style guides based on other IDE bugs, are you? That sounds pretty devastating. IDE can't see a function on a type? Better disable auto-complete.
JakeWharton
on 22 May 2019
Hi all, this is clearly a topic that developers feel really passionate about; let's just remember to keep comments constructive and relevant.
Back to the issue at hand, the simplest short term solution will be to pull this rule from the standard ruleset. We'll leave the code in the repo so that it will be easy for developers who wish to enable it for their projects can bring it in as a custom rule.
Once that is merged, there are a couple more small bugfix PRs I'd like to merge, and then we'll make a new release. (May not be until after the Memorial Day holiday.)
Thanks everyone for the input!
shashachu
on 23 May 2019
Using kotlin-android-extensions (for the synthetic view accessors) without wildcard imports is just pain.
FWIW, kotlin android extensions are explicitly allowed:
if (path != null && !path.startsWith("kotlinx.android.synthetic") && path.contains('*')) {
emit(node.startOffset, "Wildcard import", false)
}
Closing this, as the issue has reached temporary resolution.
shashachu
on 23 May 2019
@shashachu Appreciate your great effort. Would you mind to share how to add the wildcard rules back when needed? is it through Creating a ruleset? thanks.
PandaGeek1024
on 26 May 2019
@PandaGeek1024 that's correct - you'd copy in the code for it as a custom rule. Check out the Creating a ruleset section of README.md and feel free to open up an issue if you're having trouble.
shashachu
on 27 May 2019
How can we enable this at runtime with the new update? I'd like to be able to update the Ktlint Gradle and Spotless documentation on how to keep the same functionality if you want it.
JLLeitschuh
on 28 May 2019
Create a custom ruleset as per https://github.com/pinterest/ktlint#creating-a-ruleset which will contain a rule that is literally a copy paste of the code in https://github.com/pinterest/ktlint/blob/5658473d4d7aca82b187ddd3fb79c52078cb092a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/NoWildcardImportsRule.kt#L10-L22
Zhuinden
on 28 May 2019
In this issue everyone agreed that the rule should be disabled. And guess what? I see it was enabled back in the 0.34.0 release (released 2 days ago) without any references and discussions in the release notes - it's just mentioned that it was enabled back without any "why". Shall this issue be reopened?
idntfy
on 16 Jul 2019
@idntfy actually it is specifically called out in the release notes for 0.34.0: https://github.com/pinterest/ktlint/releases
The agreement was to temporarily disable the rule until there was support for globally disabling a rule, which was added in 0.34.0: https://github.com/pinterest/ktlint#custom-editorconfig-properties
shashachu
on 16 Jul 2019
Surprising! I have more belief that the comment where the agreement was made (?) was just misread and some sort of miscommunication happened, because for me it seems there were much more arguments for not having it than enforcing. I'm not going to repeat all the good arguments that were already said in this issue.
The README.MD says "No configuration. Which means no decisions to make, nothing to argue about and no special files to manage. While this might sound extreme, keep in mind that ktlint tries to capture (reflect) official code style" which is now not true anymore, because to get to widespread defaults now we need to configure it.
On a scale it's really hard to adopt now when it's enabled. We have both big number of projects and even more bigger number of developers. Committing .editorconfig to all the projects or asking everyone to call --apply-to-idea-project are just no way to go, because it cannot be centralized thus hardly manageable.
The either acceptable way to make it centralized would be to provide the way to configure it through cli params, so it could be done just in one place in a parent pom lets say.
idntfy
on 16 Jul 2019
I guess it's because of https://github.com/pinterest/ktlint/pull/503/files#diff-ccd5a7f365eb13a0c3aea71168cf3bc6L38
Zhuinden
on 17 Jul 2019
Created #533 as an attempt to find a pragmatic consensus.
idntfy
on 17 Jul 2019
Surprising! I have more belief that the comment where the agreement was made (?) was just misread and some sort of miscommunication happened, because for me it seems there were much more arguments for not having it than enforcing. I'm not going to repeat all the good arguments that were already said in this issue.
The README.MD says "No configuration. Which means no decisions to make, nothing to argue about and no special files to manage. While this might sound extreme, keep in mind that ktlint tries to capture (reflect) official code style" which is now not true anymore, because to get to widespread defaults now we need to configure it.
On a scale it's really hard to adopt now when it's enabled. We have both big number of projects and even more bigger number of developers. Committing .editorconfig to all the projects or asking everyone to call --apply-to-idea-project are just no way to go, because it cannot be centralized thus hardly manageable.
The either acceptable way to make it centralized would be to provide the way to configure it through cli params, so it could be done just in one place in a parent pom lets say.
Sorry this came as a surprise to you; disabling the rule in 0.33.0 was always intended to be a temporary solution. I actually agree with you that we're straying from the 'No configuration' tenet, but the wider the adoption of the tool, the more difficult it will become that everyone agrees on every rule. When we took over the project, Stanley said it was always his intention to support some way to globally disable rules.
shashachu
on 17 Jul 2019
Related issues
filipedelimabrito
路
3Comments
DanteAndroid
路
3Comments
pkubowicz
路
4Comments
tonilopezmr
路
4Comments
Tapchicoma
路
3Comments
Most helpful comment
Just to drop my 2 cents here:
I don't get why wildcard imports are considered to be evil. If they were, Kotlin would not copy them from Java - they seem to have dropped a lot of other baggage.
Also some inconsistencies:
Since this linter is based on both style guides, there is a conflict here and I'd personally err towards abandoning that check. and leaving it for the IDE to manage.