Skip to content

Commit

Permalink
Re-apply pinterest#1129 with fixes for regressions (pinterest#1201)
Browse files Browse the repository at this point in the history 
* Fix regressions introduced in pinterest#1129

- Used imports state has to be reset per-file
- Allow repeated imports to be auto-corrected

* fix lint

* Fix bad merge conflict

* more merge fixes
  • Loading branch information
@shashachu
shashachu committed Aug 6, 2021
1 parent b0e4a42 commit 29e3ef5
Show file tree
Hide file tree
Showing 2 changed files with 204 additions and 1 deletion.
13 changes: 12 additions & 1 deletion ...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 @@ -19,6 +19,7 @@ import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtDotQualifiedExpression
import org.jetbrains.kotlin.psi.KtImportDirective
import org.jetbrains.kotlin.psi.KtPackageDirective
import org.jetbrains.kotlin.resolve.ImportPath

class NoUnusedImportsRule : Rule("no-unused-imports") {

Expand Down Expand Up @@ -70,6 +71,7 @@ class NoUnusedImportsRule : Rule("no-unused-imports") {
private val ref = mutableSetOf<Reference>()
private val parentExpressions = mutableSetOf<String>()
private val imports = mutableSetOf<String>()
private val usedImportPaths = mutableSetOf<ImportPath>()
private var packageName = ""
private var rootNode: ASTNode? = null

Expand All @@ -84,6 +86,7 @@ class NoUnusedImportsRule : Rule("no-unused-imports") {
ref.add(Reference("*", false))
parentExpressions.clear()
imports.clear()
usedImportPaths.clear()
node.visit { vnode ->
val psi = vnode.psi
val type = vnode.elementType
Expand All @@ -99,7 +102,15 @@ class NoUnusedImportsRule : Rule("no-unused-imports") {
) {
ref.add(Reference(text.removeBackticks(), psi.parentDotQualifiedExpression() != null))
} else if (type == IMPORT_DIRECTIVE) {
imports += (vnode.psi as KtImportDirective).importPath!!.pathStr.removeBackticks().trim()
val importPath = (vnode.psi as KtImportDirective).importPath!!
if (!usedImportPaths.add(importPath)) {
emit(vnode.startOffset, "Unused import", true)
if (autoCorrect) {
vnode.psi.delete()
}
} else {
imports += importPath.pathStr.removeBackticks().trim()
}
}
}
val directCalls = ref.filter { !it.inDotQualifiedExpression }.map { it.text }
Expand Down
192 changes: 192 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 @@ -192,6 +192,168 @@ class NoUnusedImportsRuleTest {
)
}

@Test
fun testRepeatedImport() {
assertThat(
NoUnusedImportsRule().lint(
"""
import org.repository.RepositoryPolicy
import org.repository.any
import org.repository.all
import org.repository.any
fun main() {
RepositoryPolicy(
any(false), all("trial")
)
}
""".trimIndent()
)
).isEqualTo(
listOf(
LintError(4, 1, "no-unused-imports", "Unused import")
)
)

assertThat(
NoUnusedImportsRule().format(
"""
import org.repository.RepositoryPolicy
import org.repository.any
import org.repository.all
import org.repository.any
fun main() {
RepositoryPolicy(
any(false), all("trial")
)
}
""".trimIndent()
)
).isEqualTo(
"""
import org.repository.RepositoryPolicy
import org.repository.any
import org.repository.all
fun main() {
RepositoryPolicy(
any(false), all("trial")
)
}
""".trimIndent()
)
}

@Test
fun testRepeatedImportTwice() {
assertThat(
NoUnusedImportsRule().lint(
"""
import org.repository.RepositoryPolicy
import org.repository.any
import org.repository.all
import org.repository.any
import org.repository.any
fun main() {
RepositoryPolicy(
any(false), all("trial")
)
}
""".trimIndent()
)
).isEqualTo(
listOf(
LintError(4, 1, "no-unused-imports", "Unused import"),
LintError(5, 1, "no-unused-imports", "Unused import")
)
)

assertThat(
NoUnusedImportsRule().format(
"""
import org.repository.RepositoryPolicy
import org.repository.any
import org.repository.all
import org.repository.any
import org.repository.any
fun main() {
RepositoryPolicy(
any(false), all("trial")
)
}
""".trimIndent()
)
).isEqualTo(
"""
import org.repository.RepositoryPolicy
import org.repository.any
import org.repository.all
fun main() {
RepositoryPolicy(
any(false), all("trial")
)
}
""".trimIndent()
)
}

@Test
fun testRepeatedImportsWithDistinctIncidences() {
assertThat(
NoUnusedImportsRule().lint(
"""
import org.repository.RepositoryPolicy
import org.repository.any
import org.repository.all
import org.repository.many
import org.repository.any
import org.repository.none
import org.repository.all
fun main() {
RepositoryPolicy(
any(false), all("trial"), many("hello"), none("goodbye")
)
}
""".trimIndent()
)
).isEqualTo(
listOf(
LintError(5, 1, "no-unused-imports", "Unused import"),
LintError(7, 1, "no-unused-imports", "Unused import")
)
)

assertThat(
NoUnusedImportsRule().format(
"""
import org.repository.RepositoryPolicy
import org.repository.any
import org.repository.all
import org.repository.many
import org.repository.any
import org.repository.none
import org.repository.all
fun main() {
RepositoryPolicy(
any(false), all("trial"), many("hello"), none("goodbye")
)
}
""".trimIndent()
)
).isEqualTo(
"""
import org.repository.RepositoryPolicy
import org.repository.any
import org.repository.all
import org.repository.many
import org.repository.none
fun main() {
RepositoryPolicy(
any(false), all("trial"), many("hello"), none("goodbye")
)
}
""".trimIndent()
)
}

@Test
fun testFormat() {
assertThat(
Expand Down Expand Up @@ -647,4 +809,34 @@ class NoUnusedImportsRuleTest {
)
).isEmpty()
}

@Test
fun `repeated invocations on the same rule instance should work`() {
val rule = NoUnusedImportsRule()
assertThat(
rule.lint(
"""
import foo.bar
fun main() {
bar()
}
""".trimIndent()
)
).isEmpty()

assertThat(
rule.lint(
"""
import foo.bar
import foo.baz
fun main() {
bar()
baz()
}
""".trimIndent()
)
).isEmpty()
}
}

0 comments on commit 29e3ef5

Please sign in to comment.