Skip to content

Commit

Permalink
Revert removal of unused wildcard imports (#1256) as it can lead to r…
Browse files Browse the repository at this point in the history
…emoval of required imports (#1402)

There is no reliable way to determined whether a wildcard import is actually used
in the file. The AST does not seem to contain information about the actuall class
that an identifier refers to. It is preferred to not remove unused imports than
that needed imports are removed.

Revert #1256
Closes #1277
Closes #1393
  • Loading branch information
paul-dingemans authored Mar 12, 2022
1 parent 68990c8 commit 22e4234
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 8 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ This section is applicable when providing rules that depend on one or more value
- Do not remove trailing comma after a parameter of type array in an annotation (experimental:trailing-comma) ([#1379](https://github.com/pinterest/ktlint/issues/1379))
- Do not delete blank lines in KDoc (no-trailing-spaces) ([#1376](https://github.com/pinterest/ktlint/issues/1376))
- Do not indent raw string literals that are not followed by either trimIndent() or trimMargin() (`indent`) ([#1375](https://github.com/pinterest/ktlint/issues/1375))
- Revert remove unnecessary wildcard imports as introduced in Ktlint 0.43.0 (`no-unused-imports`) ([#1277](https://github.com/pinterest/ktlint/issues/1277)), ([#1393](https://github.com/pinterest/ktlint/issues/1393)), ([#1256](https://github.com/pinterest/ktlint/issues/1256))

### Changed
- Print the rule id always in the PlainReporter ([#1121](https://github.com/pinterest/ktlint/issues/1121))
Expand Down Expand Up @@ -85,7 +86,7 @@ Please welcome [paul-dingemans](https://github.com/paul-dingemans) as an officia
- Do not check for `.idea` folder presence when using `applyToIDEA` globally ([#1186](https://github.com/pinterest/ktlint/issues/1186))
- Remove spaces before primary constructor (`paren-spacing`) ([#1207](https://github.com/pinterest/ktlint/issues/1207))
- Fix false positive for delegated properties with a lambda argument (`indent`) ([#1210](https://github.com/pinterest/ktlint/issues/1210))
- Remove unnecessary wildcard imports (`no-unused-imports`) ([#1256](https://github.com/pinterest/ktlint/issues/1256))
- (REVERTED in Ktlint 0.45.0) Remove unnecessary wildcard imports (`no-unused-imports`) ([#1256](https://github.com/pinterest/ktlint/issues/1256))
- Fix indentation of KDoc comment when using tab indentation style (`indent`) ([#850](https://github.com/pinterest/ktlint/issues/850))
### Changed
- Support absolute paths for globs ([#1131](https://github.com/pinterest/ktlint/issues/1131))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import org.jetbrains.kotlin.psi.KtDotQualifiedExpression
import org.jetbrains.kotlin.psi.KtImportDirective
import org.jetbrains.kotlin.psi.KtPackageDirective
import org.jetbrains.kotlin.resolve.ImportPath
import org.jetbrains.kotlin.util.removeSuffixIfPresent

class NoUnusedImportsRule : Rule("no-unused-imports") {

Expand Down Expand Up @@ -98,7 +97,7 @@ class NoUnusedImportsRule : Rule("no-unused-imports") {
parentExpressions.add(text.substringBeforeLast("("))
}
if (type == KDocTokens.MARKDOWN_LINK && psi is KDocLink) {
val linkText = psi.getLinkText().removeBackticksAndWildcards()
val linkText = psi.getLinkText().removeBackticks()
ref.add(Reference(linkText.split('.').first(), false))
ref.add(Reference(linkText.split('.').last(), false))
} else if ((type == REFERENCE_EXPRESSION || type == OPERATION_REFERENCE) &&
Expand All @@ -113,7 +112,7 @@ class NoUnusedImportsRule : Rule("no-unused-imports") {
?.let { identifier.text }
?.takeIf { it.isNotBlank() }
?.let {
ref.add(Reference(it.removeBackticksAndWildcards(), psi.parentDotQualifiedExpression() != null))
ref.add(Reference(it.removeBackticks(), psi.parentDotQualifiedExpression() != null))
}
} else if (type == IMPORT_DIRECTIVE) {
val importPath = (vnode.psi as KtImportDirective).importPath!!
Expand All @@ -123,7 +122,7 @@ class NoUnusedImportsRule : Rule("no-unused-imports") {
vnode.psi.delete()
}
} else {
imports += importPath.pathStr.removeBackticksAndWildcards().trim()
imports += importPath.pathStr.removeBackticks().trim()
}
}
}
Expand All @@ -138,8 +137,8 @@ class NoUnusedImportsRule : Rule("no-unused-imports") {
packageName = packageDirective.qualifiedName
} else if (node.elementType == IMPORT_DIRECTIVE) {
val importDirective = node.psi as KtImportDirective
val name = importDirective.importPath?.importedName?.asString()?.removeBackticksAndWildcards()
val importPath = importDirective.importPath?.pathStr?.removeBackticksAndWildcards()!!
val name = importDirective.importPath?.importedName?.asString()?.removeBackticks()
val importPath = importDirective.importPath?.pathStr?.removeBackticks()!!
if (importDirective.aliasName == null &&
(packageName.isEmpty() || importPath.startsWith("$packageName.")) &&
importPath.substring(packageName.length + 1).indexOf('.') == -1
Expand Down Expand Up @@ -202,5 +201,5 @@ class NoUnusedImportsRule : Rule("no-unused-imports") {
return (callOrThis.parent as? KtDotQualifiedExpression)?.takeIf { it.selectorExpression == callOrThis }
}

private fun String.removeBackticksAndWildcards() = replace("`", "").removeSuffixIfPresent(".*")
private fun String.removeBackticks() = replace("`", "")
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import com.pinterest.ktlint.core.LintError
import com.pinterest.ktlint.test.format
import com.pinterest.ktlint.test.lint
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Disabled
import org.junit.jupiter.api.Test

class NoUnusedImportsRuleTest {
Expand Down Expand Up @@ -843,6 +844,10 @@ class NoUnusedImportsRuleTest {
).isEmpty()
}

// Solution for #1256 has been reverted as it can lead to removal of imports which are actually used (see test for
// #1277). For now, there seems to be no reliable way to determine whether the wildcard import is actually used or
// not.
@Disabled
@Test
fun `Issue 1256 - remove wildcard import when not used`() {
val rule = NoUnusedImportsRule()
Expand Down Expand Up @@ -894,4 +899,40 @@ class NoUnusedImportsRuleTest {
)
).isEmpty()
}

@Test
fun `Issue 1277 - Wildcard import should not be removed because it can not be reliable be determined whether it is used`() {
val rule = NoUnusedImportsRule()
assertThat(
rule.lint(
"""
import test.*
fun main() {
Test() // defined in package test
}
""".trimIndent()
)
).isEmpty()
}

@Test
fun `Issue 1393 - Wildcard import should not be removed because it can not be reliable be determined whether it is used`() {
val rule = NoUnusedImportsRule()
assertThat(
rule.lint(
"""
package com.example
import com.example.Outer.*
class Outer {
class Inner
}
val foo = Inner()
""".trimIndent()
)
).isEmpty()
}
}

0 comments on commit 22e4234

Please sign in to comment.