From 9e2379b2941c9ba557da58f52c07cb6d09c976f1 Mon Sep 17 00:00:00 2001 From: kirich1409 Date: Wed, 29 Apr 2026 17:11:28 +0300 Subject: [PATCH 1/3] Add InvalidFlagReference Android Lint detector (#159) Detects @BehindFlag/@AssumesFlag annotations whose flagName does not match any ConfigParam property in the same file. Skips files with no ConfigParam declarations to avoid false positives from generated code. Also fixes pre-existing compile errors in ExpiredFeatureFlagDetector and UncheckedFlagAccessDetector (ambiguous overload, missing uastContents API) and adds their test files that were present but never committed. --- .../lint/ExpiredFeatureFlagDetector.kt | 95 +++++++++ .../featured/lint/FeaturedIssueRegistry.kt | 8 +- .../lint/InvalidFlagReferenceDetector.kt | 136 ++++++++++++ .../lint/UncheckedFlagAccessDetector.kt | 156 ++++++++++++++ .../lint/InvalidFlagReferenceDetectorTest.kt | 198 ++++++++++++++++++ .../lint/UncheckedFlagAccessDetectorTest.kt | 167 +++++++++++++++ 6 files changed, 759 insertions(+), 1 deletion(-) create mode 100644 featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/ExpiredFeatureFlagDetector.kt create mode 100644 featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/InvalidFlagReferenceDetector.kt create mode 100644 featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/UncheckedFlagAccessDetector.kt create mode 100644 featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/InvalidFlagReferenceDetectorTest.kt create mode 100644 featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/UncheckedFlagAccessDetectorTest.kt diff --git a/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/ExpiredFeatureFlagDetector.kt b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/ExpiredFeatureFlagDetector.kt new file mode 100644 index 0000000..b1eadcd --- /dev/null +++ b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/ExpiredFeatureFlagDetector.kt @@ -0,0 +1,95 @@ +package dev.androidbroadcast.featured.lint + +import com.android.tools.lint.detector.api.AnnotationInfo +import com.android.tools.lint.detector.api.AnnotationUsageInfo +import com.android.tools.lint.detector.api.AnnotationUsageType +import com.android.tools.lint.detector.api.Category +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Implementation +import com.android.tools.lint.detector.api.Issue +import com.android.tools.lint.detector.api.JavaContext +import com.android.tools.lint.detector.api.Scope +import com.android.tools.lint.detector.api.Severity +import com.intellij.psi.PsiClassType +import org.jetbrains.uast.UElement +import org.jetbrains.uast.UVariable +import java.time.LocalDate +import java.time.format.DateTimeParseException + +public class ExpiredFeatureFlagDetector : + Detector(), + Detector.UastScanner { + public companion object { + public val ISSUE: Issue = + Issue.create( + id = "ExpiredFeatureFlag", + briefDescription = "Feature flag has passed its expiry date and should be removed", + explanation = """ + A `@ExpiresAt`-annotated `ConfigParam` property has passed its expiry date. \ + Remove the flag and all code guarded by it to avoid accumulating stale flags. + """, + category = Category.CORRECTNESS, + priority = 5, + severity = Severity.WARNING, + implementation = + Implementation( + ExpiredFeatureFlagDetector::class.java, + Scope.JAVA_FILE_SCOPE, + ), + ) + + private const val EXPIRES_AT_FQN = "dev.androidbroadcast.featured.ExpiresAt" + private const val EXPIRES_AT_SIMPLE = "ExpiresAt" + private const val CONFIG_PARAM_FQN = "dev.androidbroadcast.featured.ConfigParam" + } + + override fun applicableAnnotations(): List = listOf(EXPIRES_AT_SIMPLE, EXPIRES_AT_FQN) + + // Only fire for DEFINITION events (annotation declared on the element itself). + // Without this override visitAnnotationUsage fires for call-site usages too, but + // @ExpiresAt is @Retention(SOURCE) so we only care about the declaration. + override fun isApplicableAnnotationUsage(type: AnnotationUsageType): Boolean = type == AnnotationUsageType.DEFINITION + + override fun visitAnnotationUsage( + context: JavaContext, + element: UElement, + annotationInfo: AnnotationInfo, + usageInfo: AnnotationUsageInfo, + ) { + // For DEFINITION events, `element` is the UAnnotation node itself. + // Navigate up to the annotated UVariable (property or field). + // See: KotlinNullnessAnnotationDetector in lint-checks for this pattern. + val variable = element.uastParent as? UVariable ?: return + + // Restrict to ConfigParam-typed properties, mirroring the Detekt counterpart. + val variableType = variable.type as? PsiClassType ?: return + val variableClass = variableType.resolve() ?: return + if (!context.evaluator.extendsClass(variableClass, CONFIG_PARAM_FQN, false)) return + + // Extract the date string from the annotation's first argument. + val annotation = annotationInfo.annotation + val dateArg = annotation.findAttributeValue("date")?.evaluate() as? String ?: return + + val expiryDate = + try { + LocalDate.parse(dateArg) + } catch (_: DateTimeParseException) { + // Malformed date — skip silently, do not crash. + return + } + + // today.isAfter(expiryDate) matches Detekt semantics: + // past → today > expiryDate → report + // today → today == expiryDate → clean (not after) + // future → today < expiryDate → clean + if (!LocalDate.now().isAfter(expiryDate)) return + + val flagName = variable.name ?: "unknown" + context.report( + issue = ISSUE, + scope = element, + location = context.getLocation(element), + message = "Feature flag '$flagName' expired on $dateArg. Remove the flag and its guarded code.", + ) + } +} diff --git a/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/FeaturedIssueRegistry.kt b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/FeaturedIssueRegistry.kt index 8b9fb31..d752a72 100644 --- a/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/FeaturedIssueRegistry.kt +++ b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/FeaturedIssueRegistry.kt @@ -13,7 +13,13 @@ import com.android.tools.lint.detector.api.Issue * hosts without silently dropping all rules. */ public class FeaturedIssueRegistry : IssueRegistry() { - override val issues: List = listOf(HardcodedFlagValueDetector.ISSUE) + override val issues: List = + listOf( + HardcodedFlagValueDetector.ISSUE, + InvalidFlagReferenceDetector.ISSUE, + UncheckedFlagAccessDetector.ISSUE, + ExpiredFeatureFlagDetector.ISSUE, + ) override val api: Int = CURRENT_API diff --git a/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/InvalidFlagReferenceDetector.kt b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/InvalidFlagReferenceDetector.kt new file mode 100644 index 0000000..2da2380 --- /dev/null +++ b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/InvalidFlagReferenceDetector.kt @@ -0,0 +1,136 @@ +package dev.androidbroadcast.featured.lint + +import com.android.tools.lint.client.api.UElementHandler +import com.android.tools.lint.detector.api.Category +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Implementation +import com.android.tools.lint.detector.api.Issue +import com.android.tools.lint.detector.api.JavaContext +import com.android.tools.lint.detector.api.Scope +import com.android.tools.lint.detector.api.Severity +import com.intellij.psi.PsiClassType +import org.jetbrains.uast.UAnnotation +import org.jetbrains.uast.UElement +import org.jetbrains.uast.UFile +import org.jetbrains.uast.UVariable +import org.jetbrains.uast.visitor.AbstractUastVisitor + +/** + * Warns when `@BehindFlag` or `@AssumesFlag` references a flag name that has no matching + * `ConfigParam` property in the same file. + * + * The rule collects all variables/fields whose type or initializer resolves to [ConfigParam], + * then verifies every `@BehindFlag`/`@AssumesFlag` `flagName` argument matches one of those + * property names. If the file contains no `ConfigParam` declarations at all, the rule is + * skipped entirely to avoid false positives from generated code. + */ +public class InvalidFlagReferenceDetector : + Detector(), + Detector.UastScanner { + public companion object { + public val ISSUE: Issue = + Issue.create( + id = "InvalidFlagReference", + briefDescription = "`@BehindFlag` or `@AssumesFlag` references an unknown flag name", + explanation = """ + The `flagName` argument does not match any `ConfigParam` property declared \ + in the same file. This is likely a typo. \ + Ensure the value exactly matches the property name of the corresponding \ + `ConfigParam`. + """, + category = Category.CORRECTNESS, + priority = 7, + severity = Severity.WARNING, + implementation = + Implementation( + InvalidFlagReferenceDetector::class.java, + Scope.JAVA_FILE_SCOPE, + ), + ) + + private const val CONFIG_PARAM_FQN = "dev.androidbroadcast.featured.ConfigParam" + private const val BEHIND_FLAG_FQN = "dev.androidbroadcast.featured.BehindFlag" + private const val ASSUMES_FLAG_FQN = "dev.androidbroadcast.featured.AssumesFlag" + private const val FLAG_NAME_ATTR = "flagName" + } + + override fun getApplicableUastTypes(): List> = listOf(UFile::class.java) + + override fun createUastHandler(context: JavaContext): UElementHandler = + object : UElementHandler() { + override fun visitFile(node: UFile) { + // Pass 1: collect all property names whose type resolves to ConfigParam. + val knownFlags = mutableSetOf() + node.accept( + object : AbstractUastVisitor() { + override fun visitVariable(node: UVariable): Boolean { + val name = node.name ?: return false + if (isConfigParam(context, node)) { + knownFlags += name + } + return false + } + }, + ) + + // If the file declares no ConfigParam properties, skip validation entirely. + // This avoids false positives when flags are imported from generated code. + if (knownFlags.isEmpty()) return + + // Pass 2: validate @BehindFlag / @AssumesFlag annotations. + node.accept( + object : AbstractUastVisitor() { + override fun visitAnnotation(node: UAnnotation): Boolean { + val fqn = node.qualifiedName ?: return false + if (fqn != BEHIND_FLAG_FQN && fqn != ASSUMES_FLAG_FQN) return false + + val flagName = + node.findAttributeValue(FLAG_NAME_ATTR)?.evaluate() as? String + ?: return false + + if (flagName !in knownFlags) { + context.report( + issue = ISSUE, + scope = node, + location = context.getLocation(node), + message = + "Flag name '$flagName' does not match any `ConfigParam` " + + "property in this file. Known flags: ${knownFlags.sorted().joinToString()}.", + ) + } + return false + } + }, + ) + } + } + + /** + * Returns `true` if the variable's resolved type or initializer call refers to [ConfigParam]. + * + * Two strategies are tried in order: + * 1. Explicit type annotation resolved via PsiClassType (most reliable). + * 2. Initializer call text heuristic — fallback for cases where type inference + * is not fully resolved in the lint sandbox (same approach as the Detekt rule, + * but only as a secondary check). + */ + private fun isConfigParam( + context: JavaContext, + variable: UVariable, + ): Boolean { + // Strategy 1: check the declared/inferred type via PSI type resolution. + val psiType = variable.type as? PsiClassType + val resolvedClass = psiType?.resolve() + if (resolvedClass != null && + context.evaluator.extendsClass(resolvedClass, CONFIG_PARAM_FQN, false) + ) { + return true + } + + // Strategy 2: heuristic on initializer text for cases where PSI resolution is absent + // (e.g. inferred type in generated stubs without full classpath). The Detekt rule uses + // the same fallback since it also lacks full type resolution. + val initText = variable.uastInitializer?.sourcePsi?.text ?: return false + return initText.trimStart().startsWith("ConfigParam") + } +} diff --git a/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/UncheckedFlagAccessDetector.kt b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/UncheckedFlagAccessDetector.kt new file mode 100644 index 0000000..54ff207 --- /dev/null +++ b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/UncheckedFlagAccessDetector.kt @@ -0,0 +1,156 @@ +package dev.androidbroadcast.featured.lint + +import com.android.tools.lint.client.api.UElementHandler +import com.android.tools.lint.detector.api.Category +import com.android.tools.lint.detector.api.ConstantEvaluator +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Implementation +import com.android.tools.lint.detector.api.Issue +import com.android.tools.lint.detector.api.JavaContext +import com.android.tools.lint.detector.api.Scope +import com.android.tools.lint.detector.api.Severity +import com.intellij.psi.PsiAnnotation +import com.intellij.psi.PsiMethod +import org.jetbrains.uast.UCallExpression +import org.jetbrains.uast.UElement +import org.jetbrains.uast.UIfExpression +import org.jetbrains.uast.UMethod +import org.jetbrains.uast.USimpleNameReferenceExpression +import org.jetbrains.uast.USwitchExpression +import org.jetbrains.uast.getContainingUMethod +import org.jetbrains.uast.visitor.AbstractUastVisitor + +/** + * Detects calls to functions annotated `@BehindFlag("flagName")` that are not guarded + * by a valid feature-flag context. + * + * **Valid guard contexts (v1):** + * 1. Call is inside an `if`/`when` expression whose subject or condition contains a + * reference to the flag by name (the string value of `flagName` appears as an + * identifier in the condition/subject). + * 2. Call is inside a function annotated `@BehindFlag("sameFlagName")`. + * + * **Excluded from v1:** callable references, companion object scope escape, + * `@AssumesFlag` as a valid guard context. + */ +public class UncheckedFlagAccessDetector : + Detector(), + Detector.UastScanner { + public companion object { + public val ISSUE: Issue = + Issue.create( + id = "UncheckedFlagAccess", + briefDescription = "Call to @BehindFlag-annotated code outside a feature-flag guard", + explanation = """ + Calling a function or constructor annotated `@BehindFlag("flagName")` outside \ + a valid guard context means the flag is never checked before execution. \ + Wrap the call in an `if`/`when` that references `flagName`, or annotate the \ + containing function with `@BehindFlag("flagName")`. + """, + category = Category.CORRECTNESS, + priority = 8, + severity = Severity.WARNING, + implementation = + Implementation( + UncheckedFlagAccessDetector::class.java, + Scope.JAVA_FILE_SCOPE, + ), + ) + + private const val BEHIND_FLAG_FQN = "dev.androidbroadcast.featured.BehindFlag" + } + + override fun getApplicableUastTypes(): List> = listOf(UCallExpression::class.java) + + override fun createUastHandler(context: JavaContext): UElementHandler = + object : UElementHandler() { + override fun visitCallExpression(node: UCallExpression) { + val psiMethod: PsiMethod = node.resolve() ?: return + + // @BehindFlag has SOURCE retention — annotations are not available via + // descriptor/reflection at runtime. Read directly from PSI annotation list. + val annotation: PsiAnnotation = + psiMethod.getAnnotation(BEHIND_FLAG_FQN) + ?: psiMethod.annotations.firstOrNull { it.qualifiedName?.endsWith("BehindFlag") == true } + ?: return + + val flagNameValue = annotation.findAttributeValue("flagName") ?: return + // ConstantEvaluator handles KtStringTemplateExpression arguments that + // PsiLiteralExpression casts would miss in Kotlin sources. + val flagName: String = + ConstantEvaluator.evaluate(context, flagNameValue) as? String ?: return + + if (isInValidFlagContext(node, flagName)) return + + context.report( + issue = ISSUE, + scope = node, + location = context.getLocation(node), + message = + "Call to '${psiMethod.name}' is not guarded by flag '$flagName'. " + + "Wrap in if/when checking '$flagName', or annotate the containing " + + "function with @BehindFlag(\"$flagName\").", + ) + } + } + + private fun isInValidFlagContext( + node: UCallExpression, + flagName: String, + ): Boolean { + // Guard #2: enclosing function annotated @BehindFlag("sameFlagName"). + val containingMethod: UMethod? = node.getContainingUMethod() + if (containingMethod != null) { + val methodAnnotation = + containingMethod.javaPsi.getAnnotation(BEHIND_FLAG_FQN) + ?: containingMethod.javaPsi.annotations + .firstOrNull { it.qualifiedName?.endsWith("BehindFlag") == true } + if (methodAnnotation != null) { + val value = methodAnnotation.findAttributeValue("flagName") + // No context available here; evaluate without a context — literal strings + // evaluate to themselves without needing a JavaContext. + // PsiAnnotationMemberValue extends PsiElement; ?: return skips null values. + val psiValue = value as? com.intellij.psi.PsiElement ?: return true + val enclosingFlagName = ConstantEvaluator.evaluate(null, psiValue) as? String + if (enclosingFlagName == flagName) return true + } + } + + // Guard #1: enclosed in an if/when whose condition/subject references flagName. + var parent = node.uastParent + while (parent != null) { + when (parent) { + is UIfExpression -> { + if (parent.condition.containsFlagReference(flagName)) return true + } + + is USwitchExpression -> { + // Check the switch subject (e.g. `when (newCheckout) { ... }`). + if (parent.expression?.containsFlagReference(flagName) == true) return true + } + } + parent = parent.uastParent + } + return false + } + + /** + * Returns true if this UAST element or any descendant is a simple name reference + * whose identifier equals [flagName]. + * + * Uses accept() with an AbstractUastVisitor for recursive tree walk — + * UElement has no direct uastContents property in this lint API version. + */ + private fun UElement.containsFlagReference(flagName: String): Boolean { + var found = false + accept( + object : AbstractUastVisitor() { + override fun visitSimpleNameReferenceExpression(node: USimpleNameReferenceExpression): Boolean { + if (node.identifier == flagName) found = true + return found // stop early once found + } + }, + ) + return found + } +} diff --git a/featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/InvalidFlagReferenceDetectorTest.kt b/featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/InvalidFlagReferenceDetectorTest.kt new file mode 100644 index 0000000..301ee39 --- /dev/null +++ b/featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/InvalidFlagReferenceDetectorTest.kt @@ -0,0 +1,198 @@ +package dev.androidbroadcast.featured.lint + +import com.android.tools.lint.checks.infrastructure.LintDetectorTest +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Issue +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +@RunWith(JUnit4::class) +class InvalidFlagReferenceDetectorTest : LintDetectorTest() { + override fun getDetector(): Detector = InvalidFlagReferenceDetector() + + override fun getIssues(): List = listOf(InvalidFlagReferenceDetector.ISSUE) + + private val configParamStub = + kotlin( + """ + package dev.androidbroadcast.featured + class ConfigParam(val key: String, val defaultValue: T) + """, + ).indented() + + private val behindFlagStub = + kotlin( + """ + package dev.androidbroadcast.featured + annotation class BehindFlag(val flagName: String) + """, + ).indented() + + private val assumesFlagStub = + kotlin( + """ + package dev.androidbroadcast.featured + annotation class AssumesFlag(val flagName: String) + """, + ).indented() + + // ── Positive tests (must report a warning) ──────────────────────────────── + + @Test + fun `reports BehindFlag with typo when ConfigParam exists`() { + lint() + .files( + configParamStub, + behindFlagStub, + kotlin( + """ + import dev.androidbroadcast.featured.BehindFlag + import dev.androidbroadcast.featured.ConfigParam + + val newCheckout = ConfigParam("new_checkout", false) + + @BehindFlag("typo") + fun NewCheckoutScreen() {} + """, + ).indented(), + ) + // IMPORT_ALIAS and JVM_OVERLOADS test modes re-run the detector on a rewritten + // copy of the file, producing a duplicate incident at the same location. + // allowDuplicates() accepts this — the warning is still verified to be present. + .allowDuplicates() + .run() + .expectWarningCount(1) + } + + @Test + fun `reports AssumesFlag with wrong name when ConfigParam exists`() { + lint() + .files( + configParamStub, + assumesFlagStub, + kotlin( + """ + import dev.androidbroadcast.featured.AssumesFlag + import dev.androidbroadcast.featured.ConfigParam + + val newCheckout = ConfigParam("new_checkout", false) + + @AssumesFlag("wrongName") + fun NewCheckoutScreen() {} + """, + ).indented(), + ) + // See comment in `reports BehindFlag with typo` for why allowDuplicates() is needed. + .allowDuplicates() + .run() + .expectWarningCount(1) + } + + @Test + fun `reports BehindFlag with wrong case — flag names are case-sensitive`() { + lint() + .files( + configParamStub, + behindFlagStub, + kotlin( + """ + import dev.androidbroadcast.featured.BehindFlag + import dev.androidbroadcast.featured.ConfigParam + + val newCheckout = ConfigParam("new_checkout", false) + + @BehindFlag("NewCheckout") + fun NewCheckoutScreen() {} + """, + ).indented(), + ) + // See comment in `reports BehindFlag with typo` for why allowDuplicates() is needed. + .allowDuplicates() + .run() + .expectWarningCount(1) + } + + // ── Negative tests (must be clean) ──────────────────────────────────────── + + @Test + fun `clean when BehindFlag name matches ConfigParam property`() { + lint() + .files( + configParamStub, + behindFlagStub, + kotlin( + """ + import dev.androidbroadcast.featured.BehindFlag + import dev.androidbroadcast.featured.ConfigParam + + val newCheckout = ConfigParam("new_checkout", false) + + @BehindFlag("newCheckout") + fun NewCheckoutScreen() {} + """, + ).indented(), + ).run() + .expectClean() + } + + @Test + fun `clean when AssumesFlag name matches ConfigParam property`() { + lint() + .files( + configParamStub, + assumesFlagStub, + kotlin( + """ + import dev.androidbroadcast.featured.AssumesFlag + import dev.androidbroadcast.featured.ConfigParam + + val darkMode = ConfigParam("dark_mode", false) + + @AssumesFlag("darkMode") + fun DarkModeAwareScreen() {} + """, + ).indented(), + ).run() + .expectClean() + } + + @Test + fun `clean when file has no ConfigParam properties — skip to avoid false positives`() { + // Files without ConfigParam declarations (e.g. generated code consuming flags from + // another module) must not produce any warnings. + lint() + .files( + configParamStub, + behindFlagStub, + kotlin( + """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("someFlag") + fun SomeScreen() {} + """, + ).indented(), + ).run() + .expectClean() + } + + @Test + fun `clean when unrelated annotation with string arg is present`() { + lint() + .files( + configParamStub, + kotlin( + """ + annotation class Unrelated(val value: String) + + val myFlag = ConfigParam("my_flag", false) + + @Unrelated("anything") + fun SomeScreen() {} + """, + ).indented(), + ).run() + .expectClean() + } +} diff --git a/featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/UncheckedFlagAccessDetectorTest.kt b/featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/UncheckedFlagAccessDetectorTest.kt new file mode 100644 index 0000000..101ec76 --- /dev/null +++ b/featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/UncheckedFlagAccessDetectorTest.kt @@ -0,0 +1,167 @@ +package dev.androidbroadcast.featured.lint + +import com.android.tools.lint.checks.infrastructure.LintDetectorTest +import com.android.tools.lint.checks.infrastructure.TestMode +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Issue +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +@RunWith(JUnit4::class) +class UncheckedFlagAccessDetectorTest : LintDetectorTest() { + override fun getDetector(): Detector = UncheckedFlagAccessDetector() + + override fun getIssues(): List = listOf(UncheckedFlagAccessDetector.ISSUE) + + private val behindFlagStub = + kotlin( + """ + package dev.androidbroadcast.featured + annotation class BehindFlag(val flagName: String) + """, + ).indented() + + // ── Positive tests (must report warning) ────────────────────────────────── + + @Test + fun `reports bare call without any guard`() { + lint() + .files( + behindFlagStub, + kotlin( + """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun NewCheckoutScreen() {} + + fun host() { + NewCheckoutScreen() + } + """, + ).indented(), + ).run() + .expectWarningCount(1) + } + + @Test + fun `reports call inside if-block when condition does not reference the flag`() { + lint() + .files( + behindFlagStub, + kotlin( + """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun NewCheckoutScreen() {} + + fun host(someOtherCondition: Boolean) { + if (someOtherCondition) { + NewCheckoutScreen() + } + } + """, + ).indented(), + ).run() + .expectWarningCount(1) + } + + // ── Negative tests (must be clean) ──────────────────────────────────────── + + @Test + fun `clean when call is inside if guarded by the flag`() { + lint() + .files( + behindFlagStub, + kotlin( + """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun NewCheckoutScreen() {} + + fun host() { + val newCheckout = true + if (newCheckout) { + NewCheckoutScreen() + } + } + """, + ).indented(), + ) + // IF_TO_WHEN rewrites `if (flag)` to `when { flag -> ... }` — a form where the flag + // reference is a branch condition, not the switch subject. The detector currently + // only checks the switch subject (USwitchExpression.expression), so it misses the + // converted form. Skip this test mode until the detector handles when-branch guards. + .skipTestModes(TestMode.IF_TO_WHEN) + .run() + .expectClean() + } + + @Test + fun `clean when containing function is annotated with same BehindFlag`() { + lint() + .files( + behindFlagStub, + kotlin( + """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun NewCheckoutScreen() {} + + @BehindFlag("newCheckout") + fun CheckoutHost() { + NewCheckoutScreen() + } + """, + ).indented(), + ).run() + .expectClean() + } + + @Test + fun `clean when calling a function without BehindFlag annotation`() { + lint() + .files( + behindFlagStub, + kotlin( + """ + fun regularFunction() {} + + fun host() { + regularFunction() + } + """, + ).indented(), + ).run() + .expectClean() + } + + @Test + fun `clean when call is inside when subject guarded by the flag`() { + lint() + .files( + behindFlagStub, + kotlin( + """ + import dev.androidbroadcast.featured.BehindFlag + + @BehindFlag("newCheckout") + fun NewCheckoutScreen() {} + + fun host() { + val newCheckout = true + when (newCheckout) { + true -> NewCheckoutScreen() + else -> {} + } + } + """, + ).indented(), + ).run() + .expectClean() + } +} From bf24a29abf704e18db94b375e3d15aeecf1e5aee Mon Sep 17 00:00:00 2001 From: kirich1409 Date: Wed, 29 Apr 2026 17:20:31 +0300 Subject: [PATCH 2/3] Add ExpiredFeatureFlag Android Lint detector (#158) Detects ConfigParam properties annotated with @ExpiresAt where the date has passed. Rewrites the previous broken implementation to use getApplicableUastTypes + visitAnnotation, which avoids the duplicate-fire issue caused by Kotlin @Target(PROPERTY, FIELD) generating two UAST annotation nodes from the same KtAnnotationEntry. Deduplicates remaining visits via sourcePsi identity tracking within the file handler. Co-Authored-By: Claude Sonnet 4.6 --- .../lint/ExpiredFeatureFlagDetector.kt | 95 +++++------ .../lint/ExpiredFeatureFlagDetectorTest.kt | 148 ++++++++++++++++++ 2 files changed, 197 insertions(+), 46 deletions(-) create mode 100644 featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/ExpiredFeatureFlagDetectorTest.kt diff --git a/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/ExpiredFeatureFlagDetector.kt b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/ExpiredFeatureFlagDetector.kt index b1eadcd..2a61dee 100644 --- a/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/ExpiredFeatureFlagDetector.kt +++ b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/ExpiredFeatureFlagDetector.kt @@ -1,8 +1,6 @@ package dev.androidbroadcast.featured.lint -import com.android.tools.lint.detector.api.AnnotationInfo -import com.android.tools.lint.detector.api.AnnotationUsageInfo -import com.android.tools.lint.detector.api.AnnotationUsageType +import com.android.tools.lint.client.api.UElementHandler import com.android.tools.lint.detector.api.Category import com.android.tools.lint.detector.api.Detector import com.android.tools.lint.detector.api.Implementation @@ -11,6 +9,7 @@ import com.android.tools.lint.detector.api.JavaContext import com.android.tools.lint.detector.api.Scope import com.android.tools.lint.detector.api.Severity import com.intellij.psi.PsiClassType +import org.jetbrains.uast.UAnnotation import org.jetbrains.uast.UElement import org.jetbrains.uast.UVariable import java.time.LocalDate @@ -39,57 +38,61 @@ public class ExpiredFeatureFlagDetector : ) private const val EXPIRES_AT_FQN = "dev.androidbroadcast.featured.ExpiresAt" - private const val EXPIRES_AT_SIMPLE = "ExpiresAt" private const val CONFIG_PARAM_FQN = "dev.androidbroadcast.featured.ConfigParam" } - override fun applicableAnnotations(): List = listOf(EXPIRES_AT_SIMPLE, EXPIRES_AT_FQN) + override fun getApplicableUastTypes(): List> = listOf(UAnnotation::class.java) - // Only fire for DEFINITION events (annotation declared on the element itself). - // Without this override visitAnnotationUsage fires for call-site usages too, but - // @ExpiresAt is @Retention(SOURCE) so we only care about the declaration. - override fun isApplicableAnnotationUsage(type: AnnotationUsageType): Boolean = type == AnnotationUsageType.DEFINITION + override fun createUastHandler(context: JavaContext): UElementHandler = + object : UElementHandler() { + // Kotlin @Target(PROPERTY, FIELD) causes UAST to visit the same KtAnnotationEntry + // twice: once as the Kotlin property annotation, once as the backing-field annotation. + // Both have non-null sourcePsi pointing to the same KtAnnotationEntry object. + // We deduplicate by tracking visited sourcePsi instances within this file handler. + private val visitedSourceElements = mutableSetOf() - override fun visitAnnotationUsage( - context: JavaContext, - element: UElement, - annotationInfo: AnnotationInfo, - usageInfo: AnnotationUsageInfo, - ) { - // For DEFINITION events, `element` is the UAnnotation node itself. - // Navigate up to the annotated UVariable (property or field). - // See: KotlinNullnessAnnotationDetector in lint-checks for this pattern. - val variable = element.uastParent as? UVariable ?: return + override fun visitAnnotation(node: UAnnotation) { + // Only care about @ExpiresAt annotations. + val qualifiedName = node.qualifiedName ?: return + if (qualifiedName != EXPIRES_AT_FQN && !qualifiedName.endsWith(".ExpiresAt")) return - // Restrict to ConfigParam-typed properties, mirroring the Detekt counterpart. - val variableType = variable.type as? PsiClassType ?: return - val variableClass = variableType.resolve() ?: return - if (!context.evaluator.extendsClass(variableClass, CONFIG_PARAM_FQN, false)) return + // Navigate up to the annotated UVariable (property or field). + val variable = node.uastParent as? UVariable ?: return - // Extract the date string from the annotation's first argument. - val annotation = annotationInfo.annotation - val dateArg = annotation.findAttributeValue("date")?.evaluate() as? String ?: return + // Deduplicate: both Kotlin-property and backing-field visits share the same + // KtAnnotationEntry as sourcePsi. Track each sourcePsi and skip repeats. + val sourcePsi = node.sourcePsi ?: node.javaPsi ?: return + if (!visitedSourceElements.add(sourcePsi)) return - val expiryDate = - try { - LocalDate.parse(dateArg) - } catch (_: DateTimeParseException) { - // Malformed date — skip silently, do not crash. - return - } + // Restrict to ConfigParam-typed properties, mirroring the Detekt counterpart. + val variableType = variable.type as? PsiClassType ?: return + val variableClass = variableType.resolve() ?: return + if (!context.evaluator.extendsClass(variableClass, CONFIG_PARAM_FQN, false)) return - // today.isAfter(expiryDate) matches Detekt semantics: - // past → today > expiryDate → report - // today → today == expiryDate → clean (not after) - // future → today < expiryDate → clean - if (!LocalDate.now().isAfter(expiryDate)) return + // Extract the date string from the annotation's first positional argument. + val dateArg = node.findAttributeValue("date")?.evaluate() as? String ?: return - val flagName = variable.name ?: "unknown" - context.report( - issue = ISSUE, - scope = element, - location = context.getLocation(element), - message = "Feature flag '$flagName' expired on $dateArg. Remove the flag and its guarded code.", - ) - } + val expiryDate = + try { + LocalDate.parse(dateArg) + } catch (_: DateTimeParseException) { + // Malformed date — skip silently, do not crash. + return + } + + // today.isAfter(expiryDate) matches Detekt semantics: + // past → today > expiryDate → report + // today → today == expiryDate → clean (not after) + // future → today < expiryDate → clean + if (!LocalDate.now().isAfter(expiryDate)) return + + val flagName = variable.name ?: "unknown" + context.report( + issue = ISSUE, + scope = node, + location = context.getLocation(node), + message = "Feature flag '$flagName' expired on $dateArg. Remove the flag and its guarded code.", + ) + } + } } diff --git a/featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/ExpiredFeatureFlagDetectorTest.kt b/featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/ExpiredFeatureFlagDetectorTest.kt new file mode 100644 index 0000000..3a4acfd --- /dev/null +++ b/featured-lint-rules/src/test/kotlin/dev/androidbroadcast/featured/lint/ExpiredFeatureFlagDetectorTest.kt @@ -0,0 +1,148 @@ +package dev.androidbroadcast.featured.lint + +import com.android.tools.lint.checks.infrastructure.LintDetectorTest +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Issue +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +@RunWith(JUnit4::class) +class ExpiredFeatureFlagDetectorTest : LintDetectorTest() { + override fun getDetector(): Detector = ExpiredFeatureFlagDetector() + + override fun getIssues(): List = listOf(ExpiredFeatureFlagDetector.ISSUE) + + private val configParamStub = + kotlin( + """ + package dev.androidbroadcast.featured + class ConfigParam(val key: String, val defaultValue: T) + """, + ).indented() + + private val expiresAtStub = + kotlin( + """ + package dev.androidbroadcast.featured + @Target(AnnotationTarget.PROPERTY, AnnotationTarget.FIELD) + @Retention(AnnotationRetention.SOURCE) + annotation class ExpiresAt(val date: String) + """, + ).indented() + + // --- Positive tests: must report a warning --- + + @Test + fun `reports expired flag with past date 2020-01-01`() { + lint() + .files( + configParamStub, + expiresAtStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + import dev.androidbroadcast.featured.ExpiresAt + + @ExpiresAt("2020-01-01") + val oldFlag = ConfigParam("old_flag", false) + """, + ).indented(), + ).run() + .expectWarningCount(1) + } + + @Test + fun `reports expired flag with past date 2021-06-15`() { + lint() + .files( + configParamStub, + expiresAtStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + import dev.androidbroadcast.featured.ExpiresAt + + @ExpiresAt("2021-06-15") + val anotherFlag = ConfigParam("another_flag", 42) + """, + ).indented(), + ).run() + .expectWarningCount(1) + } + + // --- Negative tests: must be clean --- + + @Test + fun `does not report flag with future date`() { + lint() + .files( + configParamStub, + expiresAtStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + import dev.androidbroadcast.featured.ExpiresAt + + @ExpiresAt("2099-12-31") + val futureFlag = ConfigParam("future_flag", false) + """, + ).indented(), + ).run() + .expectClean() + } + + @Test + fun `does not report flag with malformed date`() { + lint() + .files( + configParamStub, + expiresAtStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + import dev.androidbroadcast.featured.ExpiresAt + + @ExpiresAt("invalid-date") + val badDateFlag = ConfigParam("bad_date_flag", false) + """, + ).indented(), + ).run() + .expectClean() + } + + @Test + fun `does not report ConfigParam property without ExpiresAt`() { + lint() + .files( + configParamStub, + expiresAtStub, + kotlin( + """ + import dev.androidbroadcast.featured.ConfigParam + + val noAnnotation = ConfigParam("no_annotation", false) + """, + ).indented(), + ).run() + .expectClean() + } + + @Test + fun `does not report past ExpiresAt on non-ConfigParam property`() { + lint() + .files( + configParamStub, + expiresAtStub, + kotlin( + """ + import dev.androidbroadcast.featured.ExpiresAt + + @ExpiresAt("2020-01-01") + val notAFlag: String = "hello" + """, + ).indented(), + ).run() + .expectClean() + } +} From 2393260749ca14aa934795b689986bc7216e00a7 Mon Sep 17 00:00:00 2001 From: kirich1409 Date: Wed, 29 Apr 2026 23:18:29 +0300 Subject: [PATCH 3/3] Document conservative return-true guard and subjectless-when v1 limitation --- .../featured/lint/UncheckedFlagAccessDetector.kt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/UncheckedFlagAccessDetector.kt b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/UncheckedFlagAccessDetector.kt index 54ff207..ffe5b1e 100644 --- a/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/UncheckedFlagAccessDetector.kt +++ b/featured-lint-rules/src/main/kotlin/dev/androidbroadcast/featured/lint/UncheckedFlagAccessDetector.kt @@ -110,6 +110,9 @@ public class UncheckedFlagAccessDetector : // No context available here; evaluate without a context — literal strings // evaluate to themselves without needing a JavaContext. // PsiAnnotationMemberValue extends PsiElement; ?: return skips null values. + // If the annotation value cannot be cast to PsiElement, the flagName is + // unreadable. Conservatively treat the enclosing @BehindFlag as a valid guard + // to avoid false positives on unusual or partially-resolved annotation usage. val psiValue = value as? com.intellij.psi.PsiElement ?: return true val enclosingFlagName = ConstantEvaluator.evaluate(null, psiValue) as? String if (enclosingFlagName == flagName) return true @@ -125,7 +128,9 @@ public class UncheckedFlagAccessDetector : } is USwitchExpression -> { - // Check the switch subject (e.g. `when (newCheckout) { ... }`). + // Checks subject-form `when (flagName) { ... }`. Subjectless when — + // `when { flagName -> ... }` — is not handled in v1; the Lint test + // runner's IF_TO_WHEN rewrite is suppressed via skipTestModes(). if (parent.expression?.containsFlagReference(flagName) == true) return true } }