Skip to content

Commit

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

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 pinterest#1256
Closes pinterest#1277
  • Loading branch information
paul-dingemans committed Mar 12, 2022
1 parent 4079b94 commit de863cf
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 8 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
- 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)), ([#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 @@ -77,7 +78,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,20 @@ 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()
}
}

0 comments on commit de863cf

Please sign in to comment.