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 87c476d72a..c903b2bcb2 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 @@ -2,6 +2,7 @@ package com.pinterest.ktlint.ruleset.standard import com.pinterest.ktlint.core.Rule import com.pinterest.ktlint.core.ast.ElementType.BY_KEYWORD +import com.pinterest.ktlint.core.ast.ElementType.DOT_QUALIFIED_EXPRESSION import com.pinterest.ktlint.core.ast.ElementType.IMPORT_DIRECTIVE import com.pinterest.ktlint.core.ast.ElementType.OPERATION_REFERENCE import com.pinterest.ktlint.core.ast.ElementType.PACKAGE_DIRECTIVE @@ -10,6 +11,7 @@ import com.pinterest.ktlint.core.ast.isPartOf import com.pinterest.ktlint.core.ast.isRoot import com.pinterest.ktlint.core.ast.visit import org.jetbrains.kotlin.com.intellij.lang.ASTNode +import org.jetbrains.kotlin.com.intellij.psi.tree.IElementType import org.jetbrains.kotlin.kdoc.lexer.KDocTokens import org.jetbrains.kotlin.kdoc.psi.impl.KDocLink import org.jetbrains.kotlin.psi.KtImportDirective @@ -62,6 +64,7 @@ class NoUnusedImportsRule : Rule("no-unused-imports") { ) private val ref = mutableSetOf() + private val imports = mutableSetOf() private var packageName = "" private var rootNode: ASTNode? = null @@ -74,16 +77,25 @@ class NoUnusedImportsRule : Rule("no-unused-imports") { rootNode = node ref.clear() // rule can potentially be executed more than once (when formatting) ref.add("*") + imports.clear() + var parentExpression: String? = null node.visit { vnode -> val psi = vnode.psi val type = vnode.elementType + val text = vnode.text + val parentImport = checkIfExpressionHasParentImport(text, type) + parentExpression = if (parentImport) text.substringBeforeLast("(") else parentExpression if (type == KDocTokens.MARKDOWN_LINK && psi is KDocLink) { val linkText = psi.getLinkText().replace("`", "") ref.add(linkText.split('.').first()) } else if ((type == REFERENCE_EXPRESSION || type == OPERATION_REFERENCE) && !vnode.isPartOf(IMPORT_DIRECTIVE) ) { - ref.add(vnode.text.trim('`')) + ref.add(text.trim('`')) + // If redundant import is present, it is filtered from the import list + parentExpression?.let { imports.removeIf { imp -> imp.endsWith(it) } } + } else if (type == IMPORT_DIRECTIVE) { + buildNonRedundantImports(text) } } } else if (node.elementType == PACKAGE_DIRECTIVE) { @@ -101,7 +113,9 @@ class NoUnusedImportsRule : Rule("no-unused-imports") { if (autoCorrect) { importDirective.delete() } - } else if (name != null && !ref.contains(name) && !operatorSet.contains(name) && !name.isComponentN() && + } else if (name != null && (!ref.contains(name) || !isAValidImport(importPath)) && + !operatorSet.contains(name) && + !name.isComponentN() && conditionalWhitelist[importPath]?.invoke(rootNode!!) != true ) { emit(node.startOffset, "Unused import", true) @@ -112,5 +126,43 @@ class NoUnusedImportsRule : Rule("no-unused-imports") { } } + // Builds a list of imports. Takes care of having aliases in case it is assigned to imports + private fun buildNonRedundantImports(text: String) { + val element = text.split("import").last().trim() + if (element.contains(" as ")) { + imports.addAll(listOf(element.split(" as ").first().replace("`", "").trim(), element.split("as").last().trim())) + } else { + imports.add(element) + } + } + + // Checks if any static method call is present in code with the parent import present in imports list + private fun checkIfExpressionHasParentImport(text: String, type: IElementType): Boolean { + val containsMethodCall = text.split(".").last().contains("(") + return type == DOT_QUALIFIED_EXPRESSION && containsMethodCall && checkIfParentImportExists(text.substringBeforeLast("(")) + } + + // Contains list of all imports and checks if given import is present + private fun checkIfParentImportExists(text: String): Boolean { + val staticImports = imports.filter { it.endsWith(text) } + staticImports.forEach { import -> + var count = 0 + imports.forEach { + val startsWith = it.startsWith(import.substringBefore(text)) + if (startsWith) count += 1 + } + // Parent import and static import both are present + if (count > 1) { + return true + } + } + return false + } + + // Check if the import being checked is present in the filtered import list + private fun isAValidImport(importPath: String): Boolean { + return imports.contains(importPath) + } + private fun String.isComponentN() = componentNRegex.matches(this) } 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 9152356108..c8ddfb4a63 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 @@ -271,6 +271,131 @@ class NoUnusedImportsRuleTest { ) } + @Test + fun testParentPackImport() { + assertThat( + NoUnusedImportsRule().lint( + """ + import org.mockito.Mockito + import org.mockito.Mockito.withSettings + fun foo() { + Mockito.mock(String::class.java, Mockito.withSettings().defaultAnswer { }) + } + """.trimIndent() + ) + ).isEqualTo( + listOf( + LintError(2, 1, "no-unused-imports", "Unused import") + ) + ) + } + + @Test + fun testParentPackImportWithPackageNamePresent() { + assertThat( + NoUnusedImportsRule().lint( + """ + package org.tw.project + import org.mockito.Mockito + import org.mockito.Mockito.withSettings + fun foo() { + Mockito.mock(String::class.java, Mockito.withSettings().defaultAnswer { }) + } + """.trimIndent() + ) + ).isEqualTo( + listOf( + LintError(3, 1, "no-unused-imports", "Unused import") + ) + ) + } + + @Test + fun testParentPackImportWithImportNameHavingNoParentImport() { + assertThat( + NoUnusedImportsRule().lint( + """ + import org.assertj.core.util.diff.DiffUtils.diff + import org.assertj.core.util.diff.DiffUtils.generateUnifiedDiff + fun foo() { + val a = diff(2) + val diff = generateUnifiedDiff(1) + } + """.trimIndent() + ) + ).isEmpty() + } + + @Test + fun testParentImportWithStaticVariable() { + assertThat( + NoUnusedImportsRule().lint( + """ + import org.repository.RepositoryPolicy + import org.repository.RepositoryPolicy.CHECKSUM_POLICY_IGNORE + fun main() { + RepositoryPolicy( + false, "trial", + CHECKSUM_POLICY_IGNORE + ) + } + """.trimIndent() + ) + ).isEmpty() + } + + @Test + fun testParentImportWithStaticVariableImportAndParentImportUnused() { + assertThat( + NoUnusedImportsRule().lint( + """ + import org.repository.RepositoryPolicy + import org.repository.RepositoryPolicy.CHECKSUM_POLICY_IGNORE + fun main() { + val a = CHECKSUM_POLICY_IGNORE + } + """.trimIndent() + ) + ).isEqualTo( + listOf( + LintError(1, 1, "no-unused-imports", "Unused import") + ) + ) + } + + @Test + fun testFormatWhenParentImportWithStaticVariableAndMethod() { + assertThat( + NoUnusedImportsRule().format( + """ + package org.tw.project + import org.repository.RepositoryPolicy + import org.repository.RepositoryPolicy.CHECKSUM_POLICY_IGNORE + import org.mockito.Mockito + import org.mockito.Mockito.withSettings + fun foo() { + Mockito.mock(String::class.java, Mockito.withSettings().defaultAnswer { }) + } + fun main() { + val a = CHECKSUM_POLICY_IGNORE + } + """.trimIndent() + ) + ).isEqualTo( + """ + package org.tw.project + import org.repository.RepositoryPolicy.CHECKSUM_POLICY_IGNORE + import org.mockito.Mockito + fun foo() { + Mockito.mock(String::class.java, Mockito.withSettings().defaultAnswer { }) + } + fun main() { + val a = CHECKSUM_POLICY_IGNORE + } + """.trimIndent() + ) + } + @Test fun `provideDelegate is allowed if there is a by keyword`() { assertThat( @@ -316,4 +441,59 @@ class NoUnusedImportsRuleTest { ) ) } + + @Test + fun shouldNotReportUnusedWhenStaticImportIsFromAnotherPackage() { + assertThat( + NoUnusedImportsRule().lint( + """ + package com.foo + + import android.text.Spannable + import androidx.core.text.toSpannable + + fun foo(text: String): Spannable { + return text.toSpannable() + } + """.trimIndent() + ) + ).isEmpty() + } + + @Test + fun `should import alias after as and not when as is present in package name`() { + assertThat( + NoUnusedImportsRule().lint( + """ + package com.fastcompany.foo + + import com.fastcompany.common.Awesome as CommonAsset + import com.company.common.alias as CommonAsset + + class SomeConfig { + + fun fooFunction() = CommonAsset() + } + + """.trimIndent() + ) + ).isEmpty() + } + + @Test + fun `should import alias after as and not when as is present in package ending name`() { + assertThat( + NoUnusedImportsRule().lint( + """ + import com.company.common.alias as CommonAsset + + class SomeConfig { + + fun fooFunction() = CommonAsset() + } + + """.trimIndent() + ) + ).isEmpty() + } }