Skip to content

Commit

Permalink
When running format mode, the rules are first executed in parallel to…
Browse files Browse the repository at this point in the history
… find linting errors. In this process,

no unused import are found because the trailing comma is not yet added to variable "bar3". Then in the next
stage the rules are run consecutively. Nof the trailing comma rule is adding a trailing comma after the type
of variable "bar3". When the no-unused-import rule runs after the trailing-comma rule, it was incorrectly
seen as part of the type of variable "bar3" and a reference "EnumThree," (with the trailing comma was added)
which in turn resulted in not recognizing that the import of EnumThree actually was used.

Closes pinterest#1367
  • Loading branch information
Paul Dingemans committed Feb 14, 2022
1 parent f7b9996 commit 61e678d
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ package com.pinterest.ktlint.ruleset.experimental
import com.pinterest.ktlint.core.LintError
import com.pinterest.ktlint.core.api.FeatureInAlphaState
import com.pinterest.ktlint.ruleset.experimental.trailingcomma.TrailingCommaRule
import com.pinterest.ktlint.ruleset.standard.NoUnusedImportsRule
import com.pinterest.ktlint.test.EditorConfigTestRule
import com.pinterest.ktlint.test.format
import com.pinterest.ktlint.test.lint
import org.assertj.core.api.Assertions.assertThat
import org.ec4j.core.model.PropertyType
import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test

Expand Down Expand Up @@ -1043,6 +1045,64 @@ class TrailingCommaRuleTest {
.isEqualTo(autoCorrectedCode)
}

@Test
fun `Trailing comma and unused imports do not affect each other`() {
val code =
"""
package com.pinterest.ktlint
import com.pinterest.ktlint.enum.Enum
import com.pinterest.ktlint.enum.EnumThree
import com.pinterest.ktlint.enum.EnumTwo
data class TrailingCommaTest(
val foo: String,
val bar: Enum,
val bar2: EnumTwo,
val bar3: EnumThree
)
""".trimIndent()
val formattedCode =
"""
package com.pinterest.ktlint
import com.pinterest.ktlint.enum.Enum
import com.pinterest.ktlint.enum.EnumThree
import com.pinterest.ktlint.enum.EnumTwo
data class TrailingCommaTest(
val foo: String,
val bar: Enum,
val bar2: EnumTwo,
val bar3: EnumThree,
)
""".trimIndent()

val editorConfigFilePath = writeEditorConfigFile(
ALLOW_TRAILING_COMMA_ON_CALL_SITE,
ALLOW_TRAILING_COMMA_ON_DECLARATION_SITE
).absolutePath
val rules = listOf(TrailingCommaRule(), NoUnusedImportsRule())
assertThat(rules.lint(editorConfigFilePath, code)).containsExactly(
LintError(9, 24, "trailing-comma", "Missing trailing comma before \")\"")
)
assertThat(rules.format(editorConfigFilePath, code)).isEqualTo(formattedCode)

// When running format mode, the rules are first executed in parallel to find linting errors. In this process,
// no unused import are found because the trailing comma is not yet added to variable "bar3". Then in the next
// stage the rules are run consecutively. Now the trailing comma rule is adding a trailing comma after the type
// of variable "bar3". When the no-unused-import rule runs after the trailing-comma rule, it was incorrectly
// seen as part of the type of variable "bar3" and a reference "EnumThree," (with the trailing comma was added)
// which in turn resulted in not recognizing that the import of EnumThree actually was used.
val afterFormatLintErrors = ArrayList<LintError>()
val formatResult = rules.format(editorConfigFilePath, code, cb = { e, _ -> afterFormatLintErrors.add(e) })
assertThat(afterFormatLintErrors).isEmpty()
assertThat(formatResult).isEqualTo(formattedCode)
}

@Test
fun assertTrue() {
val ktlintOutput = "Some unused import"

assertTrue(ktlintOutput.contains("unused import"))
}

private fun writeEditorConfigFile(vararg editorConfigProperties: Pair<PropertyType<Boolean>, String>) = editorConfigTestRule
.writeToEditorConfig(
mapOf(*editorConfigProperties)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,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.IDENTIFIER
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 @@ -12,6 +13,7 @@ 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.PsiElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.CompositeElement
import org.jetbrains.kotlin.com.intellij.psi.tree.IElementType
import org.jetbrains.kotlin.kdoc.lexer.KDocTokens
import org.jetbrains.kotlin.kdoc.psi.impl.KDocLink
Expand Down Expand Up @@ -102,7 +104,17 @@ class NoUnusedImportsRule : Rule("no-unused-imports") {
} else if ((type == REFERENCE_EXPRESSION || type == OPERATION_REFERENCE) &&
!vnode.isPartOf(IMPORT_DIRECTIVE)
) {
ref.add(Reference(text.removeBackticksAndWildcards(), psi.parentDotQualifiedExpression() != null))
val identifier = if (vnode is CompositeElement) {
vnode.findChildByType(IDENTIFIER)
} else {
vnode
}
identifier
?.let { identifier.text }
?.takeIf { it.isNotBlank() }
?.let {
ref.add(Reference(it.removeBackticksAndWildcards(), psi.parentDotQualifiedExpression() != null))
}
} else if (type == IMPORT_DIRECTIVE) {
val importPath = (vnode.psi as KtImportDirective).importPath!!
if (!usedImportPaths.add(importPath)) {
Expand Down

0 comments on commit 61e678d

Please sign in to comment.