Fix MetricsPixelNumericValueDetector for compiled Kotlin classes

[marcosholgado]

Task/Issue URL: https://app.asana.com/1/137249556945/project/1202552961248957/task/1213634576464400

Description

The MetricsPixelNumericValueDetector lint rule was silently missing violations when MetricsPixel was used in modules where the class is loaded from a compiled dependency JAR — the constructor PsiMethod can't be resolved in that case, so visitConstructor never fired.

This PR adds a fallback detection path via getApplicableUastTypes/createUastHandler that triggers on all UCallExpression nodes and uses source text matching + return type filtering to identify MetricsPixel calls. Named-argument lookup was also reworked to use UNamedExpression with a Kotlin PSI fallback, making it robust regardless of argument order. All tests now skip TestMode.REORDER_ARGUMENTS to suppress a known lint test infrastructure warning caused by overlapping edits when positional args are nested inside outer named args.

Steps to test this PR

MetricsPixelNumericValueDetector

• Change SearchMetricPixelsPlugin so one of them has a value that's not a number, i.e. "test"
• Run ./gradlew :feature-toggles-impl:lint
• Lint should throw an error

UI changes

Before	After
!(Upload before screenshot)	(Upload after screenshot)

---

Note

Medium Risk
Medium risk because it changes lint detection logic to rely on UAST/PSI fallbacks and source-text matching, which could introduce false positives/negatives across Kotlin call shapes.

Overview
Fixes MetricsPixelNumericValueDetector missing violations when MetricsPixel comes from a compiled dependency by adding a fallback scan over all UCallExpressions and filtering to MetricsPixel calls when the constructor can’t be resolved.

Reworks argument extraction to be robust to named/out-of-order arguments by using UNamedExpression with a Kotlin PSI fallback, and improves error location selection by reporting on the value expression when possible.

Updates tests to reflect the real MetricsPixel signature (default type), adds coverage for out-of-order named arguments, and skips TestMode.REORDER_ARGUMENTS to avoid test infra issues.

^{Written by Cursor Bugbot for commit 693bca6. This will update automatically on new commits. Configure here.}

[marcosholgado]

• Fix MetricsPixelNumericValueDetector for compiled Kotlin classes #7953 👈 (View in Graphite)
• develop

This stack of pull requests is managed by Graphite. Learn more about stacking.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Detected violation silently dropped when findArgExpr returns null
  • I replaced the early return with a fallback to report on the call node so invalid non-numeric values are always reported even when argument UAST conversion fails.

Or push these changes by commenting:

@cursor push a7b2e2e2d8

Preview (a7b2e2e2d8)

diff --git a/lint-rules/src/main/java/com/duckduckgo/lint/MetricsPixelNumericValueDetector.kt b/lint-rules/src/main/java/com/duckduckgo/lint/MetricsPixelNumericValueDetector.kt
--- a/lint-rules/src/main/java/com/duckduckgo/lint/MetricsPixelNumericValueDetector.kt
+++ b/lint-rules/src/main/java/com/duckduckgo/lint/MetricsPixelNumericValueDetector.kt
@@ -72,8 +72,8 @@
         val valueStr = valueText.removeSurrounding("\"")
 
         if (valueStr.toIntOrNull() == null) {
-            // Report on the value argument expression.
-            val valueExpr = findArgExpr(node, "value") ?: return
+            // Prefer reporting on the value argument expression, but never drop the violation.
+            val valueExpr = findArgExpr(node, "value") ?: node
             context.report(
                 NUMERIC_VALUE_REQUIRED,
                 valueExpr,

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix prepared fixes for both issues found in the latest run.

• ✅ Fixed: Non-literal value expressions now trigger false positives
  • The detector now evaluates the value argument as a compile-time constant string and skips non-constant expressions, restoring the previous false-positive-safe behavior.
• ✅ Fixed: Out-of-order test duplicates existing test's argument order
  • The out-of-order test now places type before value so it genuinely validates named-argument lookup independent of argument position.

Or push these changes by commenting:

@cursor push 55264103c7

Preview (55264103c7)

diff --git a/lint-rules/src/main/java/com/duckduckgo/lint/MetricsPixelNumericValueDetector.kt b/lint-rules/src/main/java/com/duckduckgo/lint/MetricsPixelNumericValueDetector.kt
--- a/lint-rules/src/main/java/com/duckduckgo/lint/MetricsPixelNumericValueDetector.kt
+++ b/lint-rules/src/main/java/com/duckduckgo/lint/MetricsPixelNumericValueDetector.kt
@@ -67,14 +67,10 @@
         val typeText = findArgText(node, "type") ?: return
         if (!typeText.contains("COUNT_WHEN_IN_WINDOW") && !typeText.contains("COUNT_ALWAYS")) return
 
-        val valueText = findArgText(node, "value") ?: return
-        // Strip surrounding quotes from a string literal so we can check if it's an integer.
-        val valueStr = valueText.removeSurrounding("\"")
+        val valueExpr = findArgExpr(node, "value") ?: return
+        val valueStr = valueExpr.evaluate() as? String ?: return
 
         if (valueStr.toIntOrNull() == null) {
-            // Report on the value argument expression; fall back to the call site if the
-            // expression can't be resolved (e.g. UastFacade.convertElement fails in Path 2).
-            val valueExpr: UExpression = findArgExpr(node, "value") ?: node
             context.report(
                 NUMERIC_VALUE_REQUIRED,
                 valueExpr,

diff --git a/lint-rules/src/test/java/com/duckduckgo/lint/MetricsPixelNumericValueDetectorTest.kt b/lint-rules/src/test/java/com/duckduckgo/lint/MetricsPixelNumericValueDetectorTest.kt
--- a/lint-rules/src/test/java/com/duckduckgo/lint/MetricsPixelNumericValueDetectorTest.kt
+++ b/lint-rules/src/test/java/com/duckduckgo/lint/MetricsPixelNumericValueDetectorTest.kt
@@ -169,10 +169,10 @@
                         fun doSomething(toggle: Any) {
                             MetricsPixel(
                                 metric = "some_metric",
-                                value = "not_a_number",
+                                type = MetricType.COUNT_ALWAYS,
                                 toggle = toggle as com.duckduckgo.feature.toggles.api.Toggle,
                                 conversionWindow = listOf(ConversionWindow(0, 1)),
-                                type = MetricType.COUNT_ALWAYS,
+                                value = "not_a_number",
                             )
                         }
                     }

[anikiki]

Looks good and works as expected! 🎉