Skip to content

Commit

Permalink
Revert "Report used import when parent reference is already present (p…
Browse files Browse the repository at this point in the history 
…interest#437)" (pinterest#532)

* Revert "Report used import when parent reference is already present (pinterest#437)"

This reverts commit 238b774.

Fixes pinterest#437, re-introduces pinterest#405

* fix build
  • Loading branch information
@shashachu
shashachu authored Jul 17, 2019
1 parent fae5355 commit b548fef
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 172 deletions.
49 changes: 2 additions & 47 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,7 +2,6 @@ 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 @@ -11,7 +10,6 @@ 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 @@ -64,7 +62,6 @@ 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 @@ -77,25 +74,16 @@ 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(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)
ref.add(vnode.text.trim('`'))
}
}
} else if (node.elementType == PACKAGE_DIRECTIVE) {
Expand All @@ -113,9 +101,7 @@ class NoUnusedImportsRule : Rule("no-unused-imports") {
if (autoCorrect) {
importDirective.delete()
}
} else if (name != null && (!ref.contains(name) || !isAValidImport(importPath)) &&
!operatorSet.contains(name) &&
!name.isComponentN() &&
} else if (name != null && !ref.contains(name) && !operatorSet.contains(name) && !name.isComponentN() &&
conditionalWhitelist[importPath]?.invoke(rootNode!!) != true
) {
emit(node.startOffset, "Unused import", true)
Expand All @@ -126,36 +112,5 @@ 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 {
imports.forEach {
if (it.endsWith(text)) {
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)
}
125 changes: 0 additions & 125 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,131 +271,6 @@ 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

0 comments on commit b548fef

Please sign in to comment.