Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow unreferenced provideDelegate if there is a by keyword in the file #513

Merged
merged 2 commits into from
Jul 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,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
Expand Down Expand Up @@ -44,18 +45,39 @@ class NoUnusedImportsRule : Rule("no-unused-imports") {
// by (https://github.com/shyiko/ktlint/issues/54)
"getValue", "setValue"
)

private val conditionalWhitelist = mapOf<String, (node: ASTNode) -> Boolean>(
Pair(
// Ignore provideDelegate if there is a `by` anywhere in the file
"org.gradle.kotlin.dsl.provideDelegate",
{ rootNode ->
var hasByKeyword = false
rootNode.visit { child ->
if (child.elementType == BY_KEYWORD) {
hasByKeyword = true
return@visit
}
}
hasByKeyword
}
)
)

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

override fun visit(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit
) {
if (node.isRoot()) {
rootNode = node
ref.clear() // rule can potentially be executed more than once (when formatting)
ref.add("*")
imports.clear()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an unrelated change, but based on the comment on Line 78, it seems necessary.

var parentExpression: String? = null
node.visit { vnode ->
val psi = vnode.psi
Expand Down Expand Up @@ -92,7 +114,9 @@ class NoUnusedImportsRule : Rule("no-unused-imports") {
importDirective.delete()
}
} else if (name != null && (!ref.contains(name) || !isAValidImport(importPath)) &&
!operatorSet.contains(name) && !name.isComponentN()
!operatorSet.contains(name) &&
!name.isComponentN() &&
conditionalWhitelist[importPath]?.invoke(rootNode!!) != true
) {
emit(node.startOffset, "Unused import", true)
if (autoCorrect) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,4 +395,50 @@ class NoUnusedImportsRuleTest {
""".trimIndent()
)
}

@Test
fun `provideDelegate is allowed if there is a by keyword`() {
assertThat(
NoUnusedImportsRule().lint(
"""
import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.api.tasks.WriteProperties
import org.gradle.kotlin.dsl.getValue
import org.gradle.kotlin.dsl.provideDelegate
import org.gradle.kotlin.dsl.registering

class DumpVersionProperties : Plugin<Project> {
override fun apply(target: Project) {
with(target) {
val dumpVersionProperties by tasks.registering(WriteProperties::class) {
setProperties(mapOf("version" to "1.2.3"))
outputFile = rootDir.resolve("version.properties")
}

}
}
}
""".trimIndent()
)
).isEmpty()
}

@Test
fun `provideDelegate is not allowed without by keyword`() {
assertThat(
NoUnusedImportsRule().lint(
"""
import org.gradle.kotlin.dsl.provideDelegate

fun main() {
}
""".trimIndent()
)
).isEqualTo(
listOf(
LintError(1, 1, "no-unused-imports", "Unused import")
)
)
}
}