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

Ignore suppressions for no-unused-imports rule #2720

Merged
merged 2 commits into from
Jun 29, 2024
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
3 changes: 3 additions & 0 deletions ktlint-rule-engine-core/api/ktlint-rule-engine-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,9 @@ public abstract interface annotation class com/pinterest/ktlint/rule/engine/core
public abstract interface annotation class com/pinterest/ktlint/rule/engine/core/api/FeatureInBetaState : java/lang/annotation/Annotation {
}

public abstract interface class com/pinterest/ktlint/rule/engine/core/api/IgnoreKtlintSuppressions {
}

public final class com/pinterest/ktlint/rule/engine/core/api/IndentConfig {
public static final field Companion Lcom/pinterest/ktlint/rule/engine/core/api/IndentConfig$Companion;
public fun <init> (Lcom/pinterest/ktlint/rule/engine/core/api/IndentConfig$IndentStyle;I)V
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.pinterest.ktlint.rule.engine.core.api

/**
* Marker interface to indicate that this [Rule] should ignore any rule suppression hints. This class is not intended to be used by Custom
* RuleSetProviders. No backward compatibility is guaranteed.
*/
public interface IgnoreKtlintSuppressions
2 changes: 1 addition & 1 deletion ktlint-rule-engine/api/ktlint-rule-engine.api
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public class com/pinterest/ktlint/rule/engine/internal/rules/InternalRule : com/
public fun getVisitorModifiers ()Ljava/util/Set;
}

public final class com/pinterest/ktlint/rule/engine/internal/rules/KtlintSuppressionRule : com/pinterest/ktlint/rule/engine/internal/rules/InternalRule {
public final class com/pinterest/ktlint/rule/engine/internal/rules/KtlintSuppressionRule : com/pinterest/ktlint/rule/engine/internal/rules/InternalRule, com/pinterest/ktlint/rule/engine/core/api/IgnoreKtlintSuppressions {
public fun <init> (Ljava/util/List;)V
public fun beforeVisitChildNodes (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;Lkotlin/jvm/functions/Function3;)V
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ internal class RuleExecutionContext private constructor(
* The [suppressionLocator] can be changed during each visit of node when running [KtLintRuleEngine.format]. So a new handler is to
* be built before visiting the nodes.
*/
val suppress = suppressionLocator(node.startOffset, rule.ruleId)
val suppress = suppressionLocator(node.startOffset, rule)
if (rule.shouldContinueTraversalOfAST()) {
try {
if (rule is RuleAutocorrectApproveHandler) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package com.pinterest.ktlint.rule.engine.internal

import com.pinterest.ktlint.rule.engine.core.api.ElementType.RBRACE
import com.pinterest.ktlint.rule.engine.core.api.IgnoreKtlintSuppressions
import com.pinterest.ktlint.rule.engine.core.api.Rule
import com.pinterest.ktlint.rule.engine.core.api.RuleId
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfig
import com.pinterest.ktlint.rule.engine.core.api.nextSibling
import com.pinterest.ktlint.rule.engine.internal.SuppressionLocatorBuilder.CommentSuppressionHint.Type.BLOCK_END
import com.pinterest.ktlint.rule.engine.internal.SuppressionLocatorBuilder.CommentSuppressionHint.Type.BLOCK_START
import com.pinterest.ktlint.rule.engine.internal.rules.KTLINT_SUPPRESSION_RULE_ID
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.PsiComment
import org.jetbrains.kotlin.psi.KtAnnotated
Expand All @@ -18,7 +19,7 @@ import org.jetbrains.kotlin.psi.psiUtil.startOffset
/**
* Detects if given `ruleId` at given `offset` is suppressed.
*/
internal typealias SuppressionLocator = (offset: Int, ruleId: RuleId) -> Boolean
internal typealias SuppressionLocator = (offset: Int, rule: Rule) -> Boolean

internal object SuppressionLocatorBuilder {
/**
Expand Down Expand Up @@ -65,15 +66,13 @@ internal object SuppressionLocatorBuilder {
}

private fun toSuppressedRegionsLocator(hintsList: List<SuppressionHint>): SuppressionLocator =
{ offset, ruleId ->
if (ruleId == KTLINT_SUPPRESSION_RULE_ID) {
// The rule to detect deprecated rule directives may not be disabled itself as otherwise the directives
// will not be reported and fixed.
{ offset, rule ->
if (rule is IgnoreKtlintSuppressions) {
false
} else {
hintsList
.filter { offset in it.range }
.any { hint -> hint.disabledRuleIds.isEmpty() || hint.disabledRuleIds.contains(ruleId.value) }
.any { hint -> hint.disabledRuleIds.isEmpty() || hint.disabledRuleIds.contains(rule.ruleId.value) }
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.BLOCK_COMMENT
import com.pinterest.ktlint.rule.engine.core.api.ElementType.EOL_COMMENT
import com.pinterest.ktlint.rule.engine.core.api.ElementType.STRING_TEMPLATE
import com.pinterest.ktlint.rule.engine.core.api.ElementType.VALUE_ARGUMENT
import com.pinterest.ktlint.rule.engine.core.api.IgnoreKtlintSuppressions
import com.pinterest.ktlint.rule.engine.core.api.RuleId
import com.pinterest.ktlint.rule.engine.core.api.ifAutocorrectAllowed
import com.pinterest.ktlint.rule.engine.core.api.isWhiteSpace
Expand Down Expand Up @@ -65,7 +66,10 @@ import org.jetbrains.kotlin.utils.addToStdlib.applyIf
*/
public class KtlintSuppressionRule(
private val allowedRuleIds: List<RuleId>,
) : InternalRule("ktlint-suppression") {
) : InternalRule("ktlint-suppression"),
// The SuppressionLocatorBuilder no longer support the old ktlint suppression directives using comments. This rule may not be disabled
// in any way as it would fail to process suppressions.
IgnoreKtlintSuppressions {
private val allowedRuleIdAsStrings = allowedRuleIds.map { it.value }

private val ruleIdValidator: (String) -> Boolean = { ruleId -> allowedRuleIdAsStrings.contains(ruleId) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.pinterest.ktlint.rule.engine.internal.FormatterTags.Companion.FORMATT
import com.pinterest.ktlint.rule.engine.internal.FormatterTags.Companion.FORMATTER_TAG_ON_ENABLED_PROPERTY
import com.pinterest.ktlint.rule.engine.internal.rules.KTLINT_SUPPRESSION_RULE_ID
import com.pinterest.ktlint.ruleset.standard.rules.IndentationRule
import com.pinterest.ktlint.ruleset.standard.rules.NoUnusedImportsRule
import org.assertj.core.api.Assertions.assertThat
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.junit.jupiter.api.Nested
Expand Down Expand Up @@ -467,6 +468,38 @@ class SuppressionLocatorBuilderTest {
assertThat(actual).isEqualTo(formattedCode)
}

@Test
fun `Issue 2696 - Given an import which is only used in a block that is suppressed then do not report that import as unused`() {
val code =
"""
import bar.Bar1
import bar.Bar2
import bar.Bar3

fun foo123() {
@Suppress("ktlint")
Bar1()

// @formatter:off
Bar2()
// @formatter:on

Bar3()
}
""".trimIndent()

val actual =
lint(
code,
editorConfigOverride = EditorConfigOverride.from(FORMATTER_TAGS_ENABLED_PROPERTY to true),
ruleProviders = setOf(RuleProvider { NoUnusedImportsRule() }),
)
assertThat(actual).containsExactly(
lintError(5, 5, "standard:no-foo-identifier-standard"),
lintError(5, 5, "$NON_STANDARD_RULE_SET_ID:no-foo-identifier"),
)
}

private class NoFooIdentifierRule(
id: RuleId,
) : Rule(
Expand Down
2 changes: 1 addition & 1 deletion ktlint-ruleset-standard/api/ktlint-ruleset-standard.api
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ public final class com/pinterest/ktlint/ruleset/standard/rules/NoUnitReturnRuleK
public static final fun getNO_UNIT_RETURN_RULE_ID ()Lcom/pinterest/ktlint/rule/engine/core/api/RuleId;
}

public final class com/pinterest/ktlint/ruleset/standard/rules/NoUnusedImportsRule : com/pinterest/ktlint/ruleset/standard/StandardRule {
public final class com/pinterest/ktlint/ruleset/standard/rules/NoUnusedImportsRule : com/pinterest/ktlint/ruleset/standard/StandardRule, com/pinterest/ktlint/rule/engine/core/api/IgnoreKtlintSuppressions {
public fun <init> ()V
public fun afterVisitChildNodes (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;Lkotlin/jvm/functions/Function3;)V
public fun beforeVisitChildNodes (Lorg/jetbrains/kotlin/com/intellij/lang/ASTNode;Lkotlin/jvm/functions/Function3;)V
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import com.pinterest.ktlint.rule.engine.core.api.ElementType.IMPORT_DIRECTIVE
import com.pinterest.ktlint.rule.engine.core.api.ElementType.OPERATION_REFERENCE
import com.pinterest.ktlint.rule.engine.core.api.ElementType.PACKAGE_DIRECTIVE
import com.pinterest.ktlint.rule.engine.core.api.ElementType.REFERENCE_EXPRESSION
import com.pinterest.ktlint.rule.engine.core.api.IgnoreKtlintSuppressions
import com.pinterest.ktlint.rule.engine.core.api.RuleId
import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint
import com.pinterest.ktlint.rule.engine.core.api.SinceKtlint.Status.STABLE
Expand Down Expand Up @@ -37,7 +38,10 @@ import org.jetbrains.kotlin.psi.KtPackageDirective
import org.jetbrains.kotlin.resolve.ImportPath

@SinceKtlint("0.2", STABLE)
public class NoUnusedImportsRule : StandardRule("no-unused-imports") {
public class NoUnusedImportsRule :
StandardRule("no-unused-imports"),
// Prevent that imports which are only used inside code that is suppressed are (falsely) reported as unused.
IgnoreKtlintSuppressions {
private val ref =
mutableSetOf(
Reference("*", false),
Expand Down