From 27e54e6520605f34313e653d6ea00ec05f258f6f Mon Sep 17 00:00:00 2001 From: Sowmya Viswanathan Date: Sat, 25 May 2019 20:11:36 +0530 Subject: [PATCH] Report used import when parent reference is already present --- .../ruleset/standard/NoUnusedImportsRule.kt | 42 ++++++++- .../standard/NoUnusedImportsRuleTest.kt | 92 +++++++++++++++++++ 2 files changed, 132 insertions(+), 2 deletions(-) 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 b3c78f03cb..1c7997705f 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 @@ -1,6 +1,7 @@ package com.pinterest.ktlint.ruleset.standard import com.pinterest.ktlint.core.Rule +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 @@ -9,6 +10,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 @@ -43,6 +45,7 @@ class NoUnusedImportsRule : Rule("no-unused-imports") { "getValue", "setValue" ) private val ref = mutableSetOf() + private val imports = mutableSetOf() private var packageName = "" override fun visit( @@ -53,16 +56,23 @@ class NoUnusedImportsRule : Rule("no-unused-imports") { if (node.isRoot()) { ref.clear() // rule can potentially be executed more than once (when formatting) ref.add("*") + 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('`')) + parentExpression?.let { imports.removeIf { imp -> imp.endsWith(it) } } + } else if (type == IMPORT_DIRECTIVE) { + buildNonRedundantImports(text) } } } else if (node.elementType == PACKAGE_DIRECTIVE) { @@ -80,7 +90,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() + ) { emit(node.startOffset, "Unused import", true) if (autoCorrect) { importDirective.delete() @@ -89,5 +101,31 @@ class NoUnusedImportsRule : Rule("no-unused-imports") { } } + private fun buildNonRedundantImports(text: String) { + val element = text.split("import").last().trim() + if (element.contains("as ")) { + imports.addAll(listOf(element.split("as").first().trim(), element.split("as").last().trim())) + } else { + imports.add(element) + } + } + + private fun checkIfExpressionHasParentImport(text: String, type: IElementType): Boolean { + val containsMethodCall = text.split(".").last().contains("(") + return type == DOT_QUALIFIED_EXPRESSION && containsMethodCall && checkIfParentImportExists(text) + } + + private fun checkIfParentImportExists(text: String): Boolean { + imports.forEach { + if (it.endsWith(text.substringBeforeLast("("))) + return true + } + return false + } + + private fun isAValidImport(importPath: String): Boolean { + return imports.contains(importPath) || imports.firstOrNull { it.replace("`", "").contains(importPath) } != null + } + 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 3390247717..05132a0d9c 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 @@ -270,4 +270,96 @@ class NoUnusedImportsRuleTest { """.trimIndent() ) } + + @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") + ) + ) + } }