From 22e423411e5de02288287de5cc0e8aa7af3879c9 Mon Sep 17 00:00:00 2001 From: paul-dingemans Date: Sat, 12 Mar 2022 17:08:24 +0100 Subject: [PATCH] Revert removal of unused wildcard imports (#1256) as it can lead to removal 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 --- CHANGELOG.md | 3 +- .../ruleset/standard/NoUnusedImportsRule.kt | 13 +++--- .../standard/NoUnusedImportsRuleTest.kt | 41 +++++++++++++++++++ 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebee20ec1f..5af2afea0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) @@ -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)) diff --git a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/NoUnusedImportsRule.kt b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/NoUnusedImportsRule.kt index 27e5c414b3..4aa1f10cc7 100644 --- a/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/NoUnusedImportsRule.kt +++ b/ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/NoUnusedImportsRule.kt @@ -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") { @@ -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) && @@ -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!! @@ -123,7 +122,7 @@ class NoUnusedImportsRule : Rule("no-unused-imports") { vnode.psi.delete() } } else { - imports += importPath.pathStr.removeBackticksAndWildcards().trim() + imports += importPath.pathStr.removeBackticks().trim() } } } @@ -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 @@ -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("`", "") } diff --git a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/NoUnusedImportsRuleTest.kt b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/NoUnusedImportsRuleTest.kt index d21b3b283e..c6af6827b4 100644 --- a/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/NoUnusedImportsRuleTest.kt +++ b/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/NoUnusedImportsRuleTest.kt @@ -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 { @@ -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() @@ -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() + } }