Skip to content

Commit

Permalink
Allow unreferenced provideDelegate if there is a by keyword in the fi…
Browse files Browse the repository at this point in the history
…le (#513)

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

Fixes #506

* fix test
  • Loading branch information
shashachu authored Jul 10, 2019
1 parent 68069a8 commit b997561
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 1 deletion.
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",

This comment has been minimized.

Copy link
@eskatos

eskatos Dec 12, 2019

Note that this isn't specific to org.gradle.kotlin.dsl! Any Kotlin library could contain such an operator function.
See https://kotlinlang.org/docs/reference/delegated-properties.html#providing-a-delegate-since-11

This comment has been minimized.

Copy link
@JakeWharton

JakeWharton Dec 12, 2019

Contributor

Yep. I have this problem right now when using clikt. Want to file a new bug? I've been meaning to for weeks...

This comment has been minimized.

Copy link
@eskatos

eskatos Dec 12, 2019

@JakeWharton here you go #669
Mind mentioning your use case in the new issue? :)

{ 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()
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")
)
)
}
}

0 comments on commit b997561

Please # to comment.