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

False negative on "import java.util.*", expected no-wildcard-imports #1792

Closed
greg-patterson opened this issue Jan 26, 2023 · 4 comments
Closed

Comments

@greg-patterson
Copy link

Expected Behavior

When java.util.* is included as a wildcard import, it should trigger the warning for no-wildcard-imports. When none of the classes in that package are used (both no-wildcard-imports and no-unused-imports rules apply), then at least one of no-wildcard-imports or no-unused-imports should be triggered.

Observed Behavior

When java.util.* is included as a wildcard import, it triggers no warnings for no-wildcard-imports. When no classes from the package are used, it also does not trigger a no-unused-imports warning.

Changing to a specific class like java.util.UUID produces the expected no-unused-imports, and a different wildcard like javax.net.* or java.time.* produces the expected no-wildcard-imports. Also, moving the java.util.* to above a different import produces the expected import-ordering.

Screenshots

Showing error case (false negative with incorrect build success):
success-java-util-wildcard-false-negative

Similar but different cases without error (correctly failing):
failure-similar-wildcard-correct-positive
failure-java-util-non-wildcard-correct-positive

Steps to Reproduce

Add java.util.* as an import in the correct ordering, run ktlint.

Your Environment

  • Version of ktlint used: 0.48.2
  • Relevant parts of the .editorconfig settings: no file, defaults only
  • Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task): Reproduced via both gradle task and command line
  • Version of Gradle used (if applicable): 7.6
  • Operating System and version: Reproduced on MacOS 12.6, Windows 10, and Alpine Linux
@paul-dingemans
Copy link
Collaborator

Can you check whether property ij_kotlin_packages_to_use_import_on_demand has been set? Imports set in this property are allowed to be used with a wildcard.

@greg-patterson
Copy link
Author

This was with no .editorconfig at all, so just using defaults.

Although, that got me looking a little deeper, and I see now that this is a feature, not a bug:

* Default IntelliJ IDEA style: Use wildcard imports for packages in "java.util", "kotlin.android.synthetic" and
* it's subpackages.
*
* https://github.com/JetBrains/kotlin/blob/ffdab473e28d0d872136b910eb2e0f4beea2e19c/idea/formatter/src/org/jetbrains/kotlin/idea/core/formatter/KotlinCodeStyleSettings.java#L81-L82
*/
defaultValue = parseAllowedWildcardImports("java.util.*,kotlinx.android.synthetic.**"),

Based on the comment, it's also part of the IntelliJ default config, and there appears to be some confusion on the JetBrains side with regard to why the exception was made in the first place, with a decision to leave it in place regardless, just for consistency.
https://youtrack.jetbrains.com/issue/KTIJ-12967

However, at some point in the past I must have disabled that particular setting in my IntelliJ config. The defaults are set in Preferences->Editor->Code Style->Kotlin, which are currently blank for me (that gets stored in .idea/codeStyles/Project.xml, in a field called PACKAGES_TO_USE_STAR_IMPORTS).

The ktlint documentation also suggests overwriting the Project.xml with a blank value for PACKAGES_TO_USE_STAR_IMPORTS:
https://pinterest.github.io/ktlint/rules/configuration-intellij-idea/

Given that, I think it makes sense that ktlint should stop including those two paths as default wildcard exceptions.

As a temporary workaround, adding an .editorconfig file to the project with just this removes those defaults:

[*.{kt,kts}]
ij_kotlin_packages_to_use_import_on_demand = nothing

Also, the second issue does appear genuine: an allowed wildcard import (due to this default, or from ij_kotlin_packages_to_use_import_on_demand) that is not used in the code, should still trigger the no-unused-imports rule.

@paul-dingemans
Copy link
Collaborator

paul-dingemans commented Jan 28, 2023

Tnx for your elaborative analysis. The feature to respect setting ij_kotlin_packages_to_use_import_on_demand was implemented not too long ago in ktlint 0.45.x via #1272. The reason for adding the default values is that they seems to be hard-coded in IntelliJ IDEA as well (https://github.com/JetBrains/kotlin/blob/ffdab473e28d0d872136b910eb2e0f4beea2e19c/idea/formatter/src/org/jetbrains/kotlin/idea/core/formatter/KotlinCodeStyleSettings.java#L81-L82).

If the property is not set explicitly, IntelliJ IDEA allows imports java.util.* and kotlinx.android.synthetic.*. If ktlint would not do the same, this would result in a conclict between ktlint and IntelliJ IDEA. Lots of users of ktlint dislike that.

Your approach by setting the property in .editorconfig is not a workaround but a good solution as it makes explicit that it is never allowed. It should be added to the documentation.

[*.{kt,kts}]
ij_kotlin_packages_to_use_import_on_demand = nothing

Currently, I am working on a new codestyle ktlint_official which will not have the limitation that it should be in sync with IntelliJ IDEA. In that codestyle, it would make sense to remove the default value. I will create a new issue for that.

Also, the second issue does appear genuine: an allowed wildcard import (due to this default, or from ij_kotlin_packages_to_use_import_on_demand) that is not used in the code, should still trigger the no-unused-imports rule.
Please see #1754 why this can/will not be fixed.

@paul-dingemans
Copy link
Collaborator

Closed as follow-up issues are created.

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

No branches or pull requests

2 participants