Skip to content

Commit

Permalink
Report unused imports when parent reference of the static import is a…
Browse files Browse the repository at this point in the history 
…lready present
  • Loading branch information
@sowmyav24
sowmyav24 committed Jul 18, 2019
1 parent 1f87128 commit 944e556
Show file tree
Hide file tree
Showing 2 changed files with 197 additions and 2 deletions.
56 changes: 54 additions & 2 deletions ...set-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/NoUnusedImportsRule.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -62,6 +64,7 @@ class NoUnusedImportsRule : Rule("no-unused-imports") {
)

private val ref = mutableSetOf<String>()
private val imports = mutableSetOf<String>()
private var packageName = ""
private var rootNode: ASTNode? = null

Expand All @@ -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) {
Expand All @@ -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)
Expand All @@ -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)
}
143 changes: 143 additions & 0 deletions ...standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/NoUnusedImportsRuleTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -316,4 +441,22 @@ 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()
}
}

0 comments on commit 944e556

Please sign in to comment.