diff --git a/buildSrc/src/main/kotlin/com/datadog/gradle/config/DetektCustomConfig.kt b/buildSrc/src/main/kotlin/com/datadog/gradle/config/DetektCustomConfig.kt index 2ef3c29781..f2b7f7f1b2 100644 --- a/buildSrc/src/main/kotlin/com/datadog/gradle/config/DetektCustomConfig.kt +++ b/buildSrc/src/main/kotlin/com/datadog/gradle/config/DetektCustomConfig.kt @@ -119,6 +119,7 @@ fun Project.detektCustomConfig() { args( "--config", "${rootDir.absolutePath}/detekt_custom_general.yml," + + "${rootDir.absolutePath}/detekt_expensive_methods.yml," + "${rootDir.absolutePath}/detekt_custom_safe_calls.yml," + "${rootDir.absolutePath}/detekt_custom_unsafe_calls.yml" ) diff --git a/dd-sdk-android-internal/api/apiSurface b/dd-sdk-android-internal/api/apiSurface index ec56c03eb9..bf723c9c63 100644 --- a/dd-sdk-android-internal/api/apiSurface +++ b/dd-sdk-android-internal/api/apiSurface @@ -78,6 +78,8 @@ class com.datadog.android.internal.lifecycle.ProcessLifecycleMonitor : android.a fun onResumed() fun onStopped() fun onPaused() +annotation com.datadog.android.internal.lint.HotMethod + constructor(String, Array) enum com.datadog.android.internal.network.GraphQLHeaders constructor(String) - DD_GRAPHQL_NAME_HEADER diff --git a/dd-sdk-android-internal/api/dd-sdk-android-internal.api b/dd-sdk-android-internal/api/dd-sdk-android-internal.api index 491233f978..124bebe56a 100644 --- a/dd-sdk-android-internal/api/dd-sdk-android-internal.api +++ b/dd-sdk-android-internal/api/dd-sdk-android-internal.api @@ -146,6 +146,11 @@ public abstract interface class com/datadog/android/internal/lifecycle/ProcessLi public abstract fun onStopped ()V } +public abstract interface annotation class com/datadog/android/internal/lint/HotMethod : java/lang/annotation/Annotation { + public abstract fun exclude ()[Ljava/lang/String; + public abstract fun message ()Ljava/lang/String; +} + public final class com/datadog/android/internal/network/GraphQLHeaders : java/lang/Enum { public static final field DD_GRAPHQL_NAME_HEADER Lcom/datadog/android/internal/network/GraphQLHeaders; public static final field DD_GRAPHQL_PAYLOAD_HEADER Lcom/datadog/android/internal/network/GraphQLHeaders; @@ -644,4 +649,3 @@ public abstract interface annotation class com/datadog/tools/annotation/NoOpImpl public abstract fun customName ()Ljava/lang/String; public abstract fun publicNoOpImplementation ()Z } - diff --git a/dd-sdk-android-internal/src/main/java/com/datadog/android/internal/lint/HotMethod.kt b/dd-sdk-android-internal/src/main/java/com/datadog/android/internal/lint/HotMethod.kt new file mode 100644 index 0000000000..7f043e1061 --- /dev/null +++ b/dd-sdk-android-internal/src/main/java/com/datadog/android/internal/lint/HotMethod.kt @@ -0,0 +1,35 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +package com.datadog.android.internal.lint + +/** + * Marks a method as a hot path — one that can be invoked on every frame or touch event. + * + * Implementations must avoid heap allocations, blocking operations, and any work + * that introduces GC pressure. The IDE/Lint will highlight annotated methods as a + * reminder to reviewers. + * + * @property message A short description of why this method is hot (e.g. "called per frame by JankStats"). + * @property exclude Check names or specific function/constructor names to skip for this site only. + * Built-in category strings: `"constructor"`, `"anonymous-object"`, `"lambda"`, + * `"string-template"`, `"factory"`, `"collection-ops"`. + * You may also pass a bare method name (e.g. `"forEach"`) or a qualified name + * (e.g. `"kotlin.collections.List.forEach"`) to exclude just that call. + * Prefer a narrow per-name exclusion over a broad category exclusion. + */ +@Retention(AnnotationRetention.SOURCE) +@Target(AnnotationTarget.FUNCTION) +annotation class HotMethod(val message: String, val exclude: Array = []) { + companion object { + const val CHECK_CONSTRUCTOR = "constructor" + const val CHECK_ANONYMOUS_OBJECT = "anonymous-object" + const val CHECK_LAMBDA = "lambda" + const val CHECK_STRING_TEMPLATE = "string-template" + const val CHECK_FACTORY = "factory" + const val CHECK_COLLECTION_OPS = "collection-ops" + } +} diff --git a/detekt_custom_safe_calls.yml b/detekt_custom_safe_calls.yml index 7b7c8805a6..e2243d6a95 100644 --- a/detekt_custom_safe_calls.yml +++ b/detekt_custom_safe_calls.yml @@ -197,6 +197,7 @@ datadog: # endregion # region Android Graphics - "android.graphics.Bitmap.recycle()" + - "android.graphics.Point.set(kotlin.Int, kotlin.Int)" - "android.graphics.Canvas.drawColor(kotlin.Int)" - "android.graphics.Canvas.drawColor(kotlin.Int, android.graphics.PorterDuff.Mode)" - "android.graphics.Canvas.drawLine(kotlin.Float, kotlin.Float, kotlin.Float, kotlin.Float, android.graphics.Paint)" diff --git a/detekt_expensive_methods.yml b/detekt_expensive_methods.yml new file mode 100644 index 0000000000..74268f9ab0 --- /dev/null +++ b/detekt_expensive_methods.yml @@ -0,0 +1,201 @@ +build: + maxIssues: 0 + +config: + validation: true + warningsAsErrors: true + +comments: + active: true + AbsentOrWrongFileLicense: + active: true + licenseTemplateFile: 'license.template' + licenseTemplateIsRegex: false + excludes: + # Gradle build outputs and generated artifacts. + - '**/build/**' + # Auto-generated Kotlin model fixtures produced by the buildSrc JSON-to-Kotlin code generator. + - '**/buildSrc/src/test/kotlin/com/example/model/**' + # Fixture files for the NoOpFactory annotation processor tests, read as text data rather than compiled. + - '**/tools/noopfactory/src/test/resources/**' + # Vendored test files ported from the upstream dd-trace-java library. + - '**/features/dd-sdk-android-trace-internal/src/test/kotlin/com/datadog/trace/**' + +complexity: + active: false + +coroutines: + active: false + +empty-blocks: + active: false + +exceptions: + active: false + +naming: + active: false + +potential-bugs: + active: false + +style: + active: false + +datadog: + active: true + excludes: + - '**/build/**' + - '**/test/**' + - '**/testDebug/**' + - '**/testRelease/**' + - '**/androidTest/**' + - '**/testFixtures/**' + - '**/buildSrc/**' + - '**/*.kts' + - '**/instrumented/**' + - '**/reliability/**' + - '**/sample/**' + - '**/tools/**' + HotMethodIllegalCall: + active: true + # Lambda literals passed to functions in `allowedInlineFunctions` are inlined by the compiler + # and do not allocate — they are excluded from the lambda-allocation check. + # Add any project-specific `inline` functions here. + allowedInlineFunctions: + # Kotlin scope functions + - 'let' + - 'also' + - 'apply' + - 'run' + - 'with' + - 'takeIf' + - 'takeUnless' + # Standard utilities + - 'synchronized' + - 'use' + - 'repeat' + - 'measureNanoTime' + - 'measureTimeMillis' + # Kotlin stdlib collection operations — ALL are inline, lambdas are not allocated + - 'filter' + - 'filterNot' + - 'filterIsInstance' + - 'filterNotNull' + - 'map' + - 'mapNotNull' + - 'mapIndexed' + - 'flatMap' + - 'flatten' + - 'forEach' + - 'forEachIndexed' + - 'any' + - 'all' + - 'none' + - 'count' + - 'find' + - 'findLast' + - 'first' + - 'firstOrNull' + - 'last' + - 'lastOrNull' + - 'single' + - 'singleOrNull' + - 'reduce' + - 'fold' + - 'foldIndexed' + - 'sumOf' + - 'maxByOrNull' + - 'minByOrNull' + - 'maxOrNull' + - 'minOrNull' + - 'groupBy' + - 'associate' + - 'associateBy' + - 'partition' + - 'sortedBy' + - 'sortedWith' + - 'sorted' + - 'toList' + - 'toMutableList' + - 'toSet' + - 'toMutableSet' + - 'toMap' + # Builder functions — inline, so the lambda body is inlined (the builder itself still allocates) + - 'buildString' + - 'buildList' + - 'buildSet' + - 'buildMap' + # Both lists use `ContainerFQN.method` format (or `SimpleClass.method` as a suffix match). + # For top-level factory functions the container is the package (e.g. kotlin.collections). + # emptyList/emptySet/emptyMap are intentionally excluded — they return cached singletons. + forbiddenFactoryFunctions: + - 'kotlin.collections.listOf' + - 'kotlin.collections.mutableListOf' + - 'kotlin.collections.arrayListOf' + - 'kotlin.collections.setOf' + - 'kotlin.collections.mutableSetOf' + - 'kotlin.collections.hashSetOf' + - 'kotlin.collections.linkedSetOf' + - 'kotlin.collections.mapOf' + - 'kotlin.collections.mutableMapOf' + - 'kotlin.collections.hashMapOf' + - 'kotlin.collections.linkedMapOf' + - 'kotlin.collections.buildList' + - 'kotlin.collections.buildSet' + - 'kotlin.collections.buildMap' + - 'kotlin.text.buildString' + forbiddenCalls: + # Linear search + - 'kotlin.collections.List.find' + - 'kotlin.collections.List.findLast' + - 'kotlin.collections.List.first' + - 'kotlin.collections.List.firstOrNull' + - 'kotlin.collections.List.last' + - 'kotlin.collections.List.lastOrNull' + - 'kotlin.collections.List.single' + - 'kotlin.collections.List.singleOrNull' + - 'kotlin.collections.List.indexOf' + - 'kotlin.collections.List.indexOfFirst' + - 'kotlin.collections.List.indexOfLast' + - 'kotlin.collections.List.lastIndexOf' + - 'kotlin.collections.List.any' + - 'kotlin.collections.List.all' + - 'kotlin.collections.List.none' + - 'kotlin.collections.List.count' + # Full iteration — allocates an iterator each call + - 'kotlin.collections.List.forEach' + - 'kotlin.collections.List.forEachIndexed' + - 'kotlin.collections.List.reduce' + - 'kotlin.collections.List.fold' + - 'kotlin.collections.List.foldIndexed' + - 'kotlin.collections.List.sumOf' + - 'kotlin.collections.List.maxOrNull' + - 'kotlin.collections.List.minOrNull' + - 'kotlin.collections.List.maxByOrNull' + - 'kotlin.collections.List.minByOrNull' + # Materialising — allocates a new collection each call + - 'kotlin.collections.List.filter' + - 'kotlin.collections.List.filterNot' + - 'kotlin.collections.List.filterIsInstance' + - 'kotlin.collections.List.filterNotNull' + - 'kotlin.collections.List.map' + - 'kotlin.collections.List.mapNotNull' + - 'kotlin.collections.List.mapIndexed' + - 'kotlin.collections.List.flatMap' + - 'kotlin.collections.List.flatten' + - 'kotlin.collections.List.toList' + - 'kotlin.collections.List.toMutableList' + - 'kotlin.collections.List.toSet' + - 'kotlin.collections.List.toMutableSet' + - 'kotlin.collections.List.toMap' + - 'kotlin.collections.List.groupBy' + - 'kotlin.collections.List.associate' + - 'kotlin.collections.List.associateBy' + - 'kotlin.collections.List.partition' + # Sorting — O(N log N) + - 'kotlin.collections.List.sorted' + - 'kotlin.collections.List.sortedBy' + - 'kotlin.collections.List.sortedWith' +datadog-test-pyramid: + active: false diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/instrumentation/gestures/GesturesDetectorWrapper.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/instrumentation/gestures/GesturesDetectorWrapper.kt index 805f89c202..26460c1a1d 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/instrumentation/gestures/GesturesDetectorWrapper.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/instrumentation/gestures/GesturesDetectorWrapper.kt @@ -12,6 +12,7 @@ package com.datadog.android.rum.internal.instrumentation.gestures import android.content.Context import android.view.MotionEvent import androidx.core.view.GestureDetectorCompat +import com.datadog.android.internal.lint.HotMethod internal class GesturesDetectorWrapper( private val gestureListener: GesturesListener, @@ -26,6 +27,7 @@ internal class GesturesDetectorWrapper( GestureDetectorCompat(context, gestureListener) ) + @HotMethod(message = "called on every touch event forwarded from WindowCallbackWrapper") fun onTouchEvent(event: MotionEvent) { if (defaultGesturesDetector.onTouchEvent(event)) { return diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/instrumentation/gestures/WindowCallbackWrapper.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/instrumentation/gestures/WindowCallbackWrapper.kt index fae8aa080b..30d6fcb079 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/instrumentation/gestures/WindowCallbackWrapper.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/instrumentation/gestures/WindowCallbackWrapper.kt @@ -12,6 +12,7 @@ import android.view.MotionEvent import android.view.Window import com.datadog.android.api.InternalLogger import com.datadog.android.api.SdkCore +import com.datadog.android.internal.lint.HotMethod import com.datadog.android.internal.utils.FixedWindowCallback import com.datadog.android.rum.GlobalRumMonitor import com.datadog.android.rum.RumActionType @@ -40,6 +41,7 @@ internal class WindowCallbackWrapper( // region Window.Callback + @HotMethod(message = "dispatched on every touch event by the system; avoid allocations inside") override fun dispatchTouchEvent(event: MotionEvent?): Boolean { if (event != null) { // we copy it and delegate it to the gesture detector for analysis @@ -50,8 +52,8 @@ internal class WindowCallbackWrapper( } catch (e: Exception) { internalLogger.log( InternalLogger.Level.ERROR, - listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY), - { "Error processing MotionEvent" }, + ERROR_LOG_TARGETS, + MESSAGE_ERROR_PROCESSING_MOTION_EVENT, e ) } finally { @@ -60,8 +62,8 @@ internal class WindowCallbackWrapper( } else { internalLogger.log( InternalLogger.Level.ERROR, - listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY), - { "Received null MotionEvent" } + ERROR_LOG_TARGETS, + MESSAGE_NULL_MOTION_EVENT ) } @@ -175,5 +177,15 @@ internal class WindowCallbackWrapper( const val BACK_DEFAULT_TARGET_NAME: String = "back" const val EVENT_CONSUMED: Boolean = true + + // Pre-allocated to avoid listOf() allocation on the touch hot path (error paths only, + // but still inside @HotMethod body so the allocation is avoided here). + internal val ERROR_LOG_TARGETS = listOf( + InternalLogger.Target.MAINTAINER, + InternalLogger.Target.TELEMETRY + ) + + private val MESSAGE_ERROR_PROCESSING_MOTION_EVENT: () -> String = { "Error processing MotionEvent" } + private val MESSAGE_NULL_MOTION_EVENT: () -> String = { "Received null MotionEvent" } } } diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/metric/slowframes/SlowFramesListener.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/metric/slowframes/SlowFramesListener.kt index b470688427..a803f8bef3 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/metric/slowframes/SlowFramesListener.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/metric/slowframes/SlowFramesListener.kt @@ -7,6 +7,7 @@ package com.datadog.android.rum.internal.metric.slowframes import android.os.Build import androidx.metrics.performance.FrameData +import com.datadog.android.internal.lint.HotMethod import com.datadog.android.internal.time.TimeProvider import com.datadog.android.rum.configuration.SlowFramesConfiguration import com.datadog.android.rum.internal.domain.FrameMetricsData @@ -64,6 +65,7 @@ internal class DefaultSlowFramesListener( } // Called from the background thread + @HotMethod(message = "invoked on every JankStats frame to track slow frames; synchronized inside") override fun onFrame(volatileFrameData: FrameData) { val viewId = currentViewId // currentViewStartedTimestampNs can be set by RUM thread in onViewCreated after we read currentViewId, @@ -109,6 +111,10 @@ internal class DefaultSlowFramesListener( // No previous slow frame record or amount of time since the last update // is significant enough to consider it idle - adding a new slow frame record. if (frameDurationNs > 0) { + // SlowFrameRecord is intentionally allocated here: a new record is only + // created when a genuinely new slow-frame segment starts (not every frame), + // so the allocation rate is bounded by actual jank events, not frame rate. + @Suppress("HotMethodIllegalCall") report.slowFramesRecords += SlowFrameRecord( frameStartedTimestampNs, frameDurationNs diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt index 010a45d9b5..a897cd7a0f 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FPSVitalListener.kt @@ -8,6 +8,7 @@ package com.datadog.android.rum.internal.vitals import android.os.Build import androidx.annotation.RequiresApi import androidx.metrics.performance.FrameData +import com.datadog.android.internal.lint.HotMethod import com.datadog.android.internal.system.BuildSdkVersionProvider import com.datadog.android.rum.internal.domain.FrameMetricsData import java.util.concurrent.TimeUnit @@ -21,6 +22,7 @@ internal class FPSVitalListener( private var frameDeadline = EXPECTED_60_FPS_FRAME_DURATION_NS private var displayRefreshRate: Double = SIXTY_FPS + @HotMethod(message = "invoked on every JankStats frame to compute FPS vital") override fun onFrame(volatileFrameData: FrameData) { val durationNs = volatileFrameData.frameDurationUiNanos if (durationNs > 0.0) { diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FrameStatesAggregator.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FrameStatesAggregator.kt index 4419f3ff71..7633df0914 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FrameStatesAggregator.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/FrameStatesAggregator.kt @@ -23,6 +23,7 @@ import androidx.annotation.RequiresApi import androidx.metrics.performance.FrameData import androidx.metrics.performance.JankStats import com.datadog.android.api.InternalLogger +import com.datadog.android.internal.lint.HotMethod import com.datadog.android.internal.system.BuildSdkVersionProvider import com.datadog.android.internal.utils.getSystemServiceAs import com.datadog.android.rum.internal.domain.FrameMetricsData @@ -154,6 +155,7 @@ internal class FrameStatesAggregator( // region JankStats.OnFrameListener + @HotMethod(message = "invoked on every JankStats frame; delegates to all frameStateListeners") override fun onFrame(volatileFrameData: FrameData) { // This method is called pretty often and forEach{} gonna create iterator instance each time. // To reduce gc pressure we use for-loop iteration here: @@ -283,6 +285,7 @@ internal class FrameStatesAggregator( @RequiresApi(Build.VERSION_CODES.N) inner class DDFrameMetricsListener : Window.OnFrameMetricsAvailableListener { + @HotMethod(message = "called by the framework on every rendered frame; avoid iterator allocations") @RequiresApi(Build.VERSION_CODES.N) override fun onFrameMetricsAvailable( window: Window, diff --git a/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/callback/RecorderWindowCallback.kt b/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/callback/RecorderWindowCallback.kt index c6fe0a1f92..a986acc064 100644 --- a/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/callback/RecorderWindowCallback.kt +++ b/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/callback/RecorderWindowCallback.kt @@ -12,6 +12,7 @@ import android.view.MotionEvent import android.view.Window import androidx.annotation.MainThread import com.datadog.android.api.InternalLogger +import com.datadog.android.internal.lint.HotMethod import com.datadog.android.internal.time.TimeProvider import com.datadog.android.internal.utils.FixedWindowCallback import com.datadog.android.internal.utils.densityNormalized @@ -53,14 +54,18 @@ internal class RecorderWindowCallback( private var lastPerformedFlushTimeInNs: Long = timeProvider.getDeviceElapsedTimeNanos() private var shouldRecordMotion: Boolean = false + // Pre-allocated and reused across ACTION_DOWN events — safe because dispatchTouchEvent is @MainThread. + private val reusableTouchLocation = Point() + // region Window.Callback + @HotMethod(message = "dispatched on every touch event; session-replay pointer recording runs here") @MainThread override fun dispatchTouchEvent(event: MotionEvent?): Boolean { if (event != null) { if (event.action == MotionEvent.ACTION_DOWN) { - val touchLocation = Point(event.x.toInt(), event.y.toInt()) - shouldRecordMotion = touchPrivacyManager.shouldRecordTouch(touchLocation) + reusableTouchLocation.set(event.x.toInt(), event.y.toInt()) + shouldRecordMotion = touchPrivacyManager.shouldRecordTouch(reusableTouchLocation) } if (shouldRecordMotion) { @@ -77,7 +82,7 @@ internal class RecorderWindowCallback( internalLogger.log( InternalLogger.Level.ERROR, InternalLogger.Target.USER, - { MOTION_EVENT_WAS_NULL_ERROR_MESSAGE }, + MESSAGE_MOTION_EVENT_WAS_NULL, null ) } @@ -211,9 +216,10 @@ internal class RecorderWindowCallback( // every 10 frames we flush the buffer internal val FLUSH_BUFFER_THRESHOLD_NS: Long = MOTION_UPDATE_DELAY_THRESHOLD_NS * 10 - internal const val MOTION_EVENT_WAS_NULL_ERROR_MESSAGE = - "RecorderWindowCallback: intercepted null motion event" internal const val FAIL_TO_PROCESS_MOTION_EVENT_ERROR_MESSAGE = "RecorderWindowCallback: wrapped callback failed to handle the motion event" + private val MESSAGE_MOTION_EVENT_WAS_NULL: () -> String = { + "RecorderWindowCallback: intercepted null motion event" + } } } diff --git a/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/listener/WindowsOnDrawListener.kt b/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/listener/WindowsOnDrawListener.kt index 9d75b12fda..f19c4d3cae 100644 --- a/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/listener/WindowsOnDrawListener.kt +++ b/features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/listener/WindowsOnDrawListener.kt @@ -12,6 +12,7 @@ import androidx.annotation.MainThread import androidx.annotation.UiThread import com.datadog.android.api.feature.FeatureSdkCore import com.datadog.android.api.feature.measureMethodCallPerf +import com.datadog.android.internal.lint.HotMethod import com.datadog.android.sessionreplay.ImagePrivacy import com.datadog.android.sessionreplay.TextAndInputPrivacy import com.datadog.android.sessionreplay.internal.TouchPrivacyManager @@ -42,6 +43,7 @@ internal class WindowsOnDrawListener( internal val weakReferencedDecorViews: List> = zOrderedDecorViews.map { WeakReference(it) } + @HotMethod(message = "triggered on every ViewTreeObserver draw pass; delegates to debounced snapshot runnable") @MainThread override fun onDraw() { debouncer.debounce(snapshotRunnable) diff --git a/integrations/dd-sdk-android-compose/src/main/kotlin/com/datadog/android/compose/internal/utils/LayoutNodeUtils.kt b/integrations/dd-sdk-android-compose/src/main/kotlin/com/datadog/android/compose/internal/utils/LayoutNodeUtils.kt index 8d95f0e8dc..2d2057ba05 100644 --- a/integrations/dd-sdk-android-compose/src/main/kotlin/com/datadog/android/compose/internal/utils/LayoutNodeUtils.kt +++ b/integrations/dd-sdk-android-compose/src/main/kotlin/com/datadog/android/compose/internal/utils/LayoutNodeUtils.kt @@ -21,6 +21,7 @@ import com.datadog.android.Datadog import com.datadog.android.api.InternalLogger import com.datadog.android.api.feature.FeatureSdkCore import com.datadog.android.compose.DatadogSemanticsPropertyKey +import com.datadog.android.internal.lint.HotMethod import com.datadog.android.rum.RumAttributes.ACTION_TARGET_ROLE import com.datadog.android.rum.RumAttributes.ACTION_TARGET_SELECTED import java.lang.reflect.Method @@ -29,6 +30,10 @@ internal class LayoutNodeUtils { private val methodResolver = MethodResolver() + // Pre-allocated and cleared on each resolveLayoutNode call to avoid mutableMapOf() allocation. + private val reusableCustomAttributes = mutableMapOf() + + @HotMethod(message = "called per LayoutNode on every tap/scroll gesture to resolve the interaction target") @Suppress("NestedBlockDepth", "CyclomaticComplexMethod") fun resolveLayoutNode(node: LayoutNode): TargetNode? { return runSafe("resolveLayoutNode") { @@ -37,7 +42,7 @@ internal class LayoutNodeUtils { var datadogTag: String? = null var role: Role? = null var selected: Boolean? = null - val customAttributes = mutableMapOf() + reusableCustomAttributes.clear() for (info in node.getModifierInfo()) { val modifier = info.modifier if (modifier is SemanticsModifier) { @@ -75,18 +80,17 @@ internal class LayoutNodeUtils { } } } - selected?.let { - customAttributes[ACTION_TARGET_SELECTED] = it - } - role?.let { - customAttributes[ACTION_TARGET_ROLE] = it - } + selected?.let { reusableCustomAttributes[ACTION_TARGET_SELECTED] = it } + role?.let { reusableCustomAttributes[ACTION_TARGET_ROLE] = it } datadogTag?.let { + // TargetNode is a return-value result type; allocation is unavoidable here. + // It is only created when a Datadog-tagged node is found (not every traversal). + @Suppress("HotMethodIllegalCall") TargetNode( tag = it, isClickable = isClickable, isScrollable = isScrollable, - customAttributes = customAttributes.toMap() + customAttributes = reusableCustomAttributes.toMap() ) } } @@ -101,6 +105,7 @@ internal class LayoutNodeUtils { } } + @HotMethod(message = "called per LayoutNode on every tap/scroll gesture for hit-testing bounds") fun getLayoutNodeBoundsInWindow(node: LayoutNode): Rect? = when (methodResolver.state) { MethodResolver.State.UNKNOWN -> { getLayoutNodeBoundsInWindowInternal(node) ?: getLayoutNodeBoundsInWindowReflection(node) @@ -133,7 +138,7 @@ internal class LayoutNodeUtils { ?.invoke(this) } - private fun runSafe(callSite: String, action: () -> T): T? { + private inline fun runSafe(callSite: String, action: () -> T): T? { try { return action() } catch (@Suppress("TooGenericExceptionCaught") e: Throwable) { diff --git a/tools/detekt/build.gradle.kts b/tools/detekt/build.gradle.kts index 9809001629..c52339ccff 100644 --- a/tools/detekt/build.gradle.kts +++ b/tools/detekt/build.gradle.kts @@ -18,11 +18,16 @@ dependencies { implementation(libs.kotlinReflect) implementation(libs.androidXAnnotation) compileOnly(libs.detektApi) + // HotMethod and its CHECK_* companion constants live in dd-sdk-android-internal. + // The constants are `const val` strings — the Kotlin compiler inlines them, so + // this module has no runtime dependency on Android classes. + compileOnly(project(":dd-sdk-android-internal")) testImplementation(libs.bundles.jUnit5) testImplementation(libs.bundles.testTools) testImplementation(libs.detektTest) testImplementation(libs.robolectric) + testImplementation(project(":dd-sdk-android-internal")) } kotlinConfig() diff --git a/tools/detekt/src/main/kotlin/com/datadog/tools/detekt/DatadogProvider.kt b/tools/detekt/src/main/kotlin/com/datadog/tools/detekt/DatadogProvider.kt index 5590504b9e..ec8f28bc07 100644 --- a/tools/detekt/src/main/kotlin/com/datadog/tools/detekt/DatadogProvider.kt +++ b/tools/detekt/src/main/kotlin/com/datadog/tools/detekt/DatadogProvider.kt @@ -7,6 +7,7 @@ package com.datadog.tools.detekt import com.datadog.tools.detekt.rules.sdk.CheckInternal +import com.datadog.tools.detekt.rules.sdk.HotMethodIllegalCall import com.datadog.tools.detekt.rules.sdk.InvalidStringFormat import com.datadog.tools.detekt.rules.sdk.PackageNameVisibility import com.datadog.tools.detekt.rules.sdk.PreferTimeProvider @@ -32,6 +33,7 @@ class DatadogProvider : RuleSetProvider { ruleSetId, listOf( CheckInternal(), + HotMethodIllegalCall(config), InvalidStringFormat(), PackageNameVisibility(config), PreferTimeProvider(config), diff --git a/tools/detekt/src/main/kotlin/com/datadog/tools/detekt/rules/sdk/HotMethodIllegalCall.kt b/tools/detekt/src/main/kotlin/com/datadog/tools/detekt/rules/sdk/HotMethodIllegalCall.kt new file mode 100644 index 0000000000..ac4fc30900 --- /dev/null +++ b/tools/detekt/src/main/kotlin/com/datadog/tools/detekt/rules/sdk/HotMethodIllegalCall.kt @@ -0,0 +1,271 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +package com.datadog.tools.detekt.rules.sdk + +import com.datadog.android.internal.lint.HotMethod +import com.datadog.tools.detekt.rules.AbstractCallExpressionRule +import io.gitlab.arturbosch.detekt.api.CodeSmell +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.api.Debt +import io.gitlab.arturbosch.detekt.api.Entity +import io.gitlab.arturbosch.detekt.api.Issue +import io.gitlab.arturbosch.detekt.api.Severity +import io.gitlab.arturbosch.detekt.api.config +import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution +import org.jetbrains.kotlin.descriptors.FunctionDescriptor +import org.jetbrains.kotlin.psi.KtAnnotationEntry +import org.jetbrains.kotlin.psi.KtCallExpression +import org.jetbrains.kotlin.psi.KtCollectionLiteralExpression +import org.jetbrains.kotlin.psi.KtLambdaExpression +import org.jetbrains.kotlin.psi.KtLiteralStringTemplateEntry +import org.jetbrains.kotlin.psi.KtNamedFunction +import org.jetbrains.kotlin.psi.KtObjectLiteralExpression +import org.jetbrains.kotlin.psi.KtStringTemplateExpression +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall + +/** + * Detects heap allocations and O(N) collection operations inside methods annotated with `@HotMethod`. + * + * Hot methods are called on every frame or touch event. Flagged patterns: + * - Constructor calls (`ArrayList()`, `StringBuilder()`, any `PascalCase()`) + * - Anonymous object expressions (`object : Runnable { ... }`) + * - Lambda / closure literals `{ }` that create a function object + * - String templates `"$variable"` that allocate a `StringBuilder` + * - Collection/builder factory functions (`mutableListOf`, `buildString`, etc.) + * - O(N) collection operations (`find`, `forEach`, `filter`, `map`, `indexOf`, etc.) + * + * All of the above allocate on every invocation, creating GC pressure and risking UI jank. + * Use index-based for-loops, pre-allocated fields, and pre-computed data instead. + * + * Entries in `forbiddenCalls` and `forbiddenFactoryFunctions` support three formats: + * - `forEach` — method name only, any receiver + * - `List.forEach` — any type whose FQN ends with `List` + * - `kotlin.collections.List.forEach` — exact FQN match + * + * `allowedInlineFunctions` lists callee names whose lambda arguments are inlined by the + * compiler and therefore do not allocate. Add any project-specific inline functions here. + * + * Configure all lists in detekt_expensive_methods.yml under `datadog > HotMethodIllegalCall`. + * + * @active + */ +@RequiresTypeResolution +class HotMethodIllegalCall( + config: Config = Config.empty +) : AbstractCallExpressionRule(config, includeTypeArguments = false) { + + override val issue: Issue = Issue( + javaClass.simpleName, + Severity.Defect, + "Heap allocations and O(N) collection operations are forbidden inside @HotMethod functions. " + + "They run on every frame or touch event and cause GC pressure. " + + "Use index-based for-loops, pre-allocated fields, and pre-computed data instead.", + Debt.TWENTY_MINS + ) + + private val forbiddenCalls: List by config(defaultValue = emptyList()) + private val forbiddenFactoryFunctions: List by config(defaultValue = emptyList()) + private val allowedInlineFunctions: List by config(defaultValue = emptyList()) + + private val forbiddenCallsSet: Set by lazy { forbiddenCalls.toSet() } + private val forbiddenFactoryFunctionsSet: Set by lazy { forbiddenFactoryFunctions.toSet() } + private val allowedInlineFunctionsSet: Set by lazy { allowedInlineFunctions.toSet() } + + // Each named function pushes its @HotMethod exclude set (null = not hot). + // Lambdas inherit the enclosing named-function context. + private val functionDepthStack = ArrayDeque?>() + + private val insideHotMethod: Boolean + get() = functionDepthStack.lastOrNull() != null + + // Returns true if ANY of the candidate strings is in the current exclude set. + // Pass the category constant first, then specific names, so a category exclusion + // catches all variants (e.g. CHECK_CONSTRUCTOR catches any constructor call). + private fun isExcluded(vararg candidates: String): Boolean { + val excluded = functionDepthStack.lastOrNull() ?: return false + return candidates.any { it in excluded } + } + + override fun visitNamedFunction(function: KtNamedFunction) { + val hotAnnotation = function.annotationEntries.find { + it.shortName?.asString() == HOT_METHOD_ANNOTATION + } + functionDepthStack.addLast(hotAnnotation?.extractExcludedChecks()) + super.visitNamedFunction(function) + functionDepthStack.removeLast() + } + + private fun KtAnnotationEntry.extractExcludedChecks(): Set { + val excludeArg = valueArguments.find { it.getArgumentName()?.asName?.asString() == "exclude" } + ?: return emptySet() + val expr = excludeArg.getArgumentExpression() ?: return emptySet() + return (expr as? KtCollectionLiteralExpression) + ?.getInnerExpressions() + ?.filterIsInstance() + ?.flatMap { it.entries.filterIsInstance() } + ?.map { it.text } + ?.toSet() + ?: emptySet() + } + + // Constructor calls (PascalCase) are detected via PSI — the type-resolved path treats some + // Java constructors differently and may not emit functionName=="constructor" reliably. + override fun visitCallExpression(expression: KtCallExpression) { + if (insideHotMethod) { + val calleeName = expression.calleeExpression?.text ?: "" + if (calleeName.isNotEmpty() && calleeName[0].isUpperCase()) { + if (!isExcluded(HotMethod.CHECK_CONSTRUCTOR, calleeName)) { + report( + CodeSmell( + issue, + Entity.from(expression), + "Constructor call `$calleeName()` inside a @HotMethod allocates a new object on every " + + "invocation. Move the allocation to a field or pre-allocate outside the hot path." + ) + ) + return + } + } + } + super.visitCallExpression(expression) + } + + override fun visitObjectLiteralExpression(expression: KtObjectLiteralExpression) { + super.visitObjectLiteralExpression(expression) + if (!insideHotMethod || isExcluded(HotMethod.CHECK_ANONYMOUS_OBJECT)) return + report( + CodeSmell( + issue, + Entity.from(expression), + "Anonymous object created inside a @HotMethod allocates a new instance on every " + + "invocation. Extract it to a field or a top-level object." + ) + ) + } + + override fun visitLambdaExpression(expression: KtLambdaExpression) { + super.visitLambdaExpression(expression) + if (!insideHotMethod || isExcluded(HotMethod.CHECK_LAMBDA)) return + if (expression.isInlinedLambda()) return + report( + CodeSmell( + issue, + Entity.from(expression), + "Lambda literal inside a @HotMethod creates a function object on every invocation. " + + "Extract it to a pre-allocated field. If it is passed to a known `inline` function," + + " add that function to `allowedInlineFunctions` in the config." + ) + ) + } + + /** + * Returns true when the lambda does not result in an object allocation: + * 1. Its containing call is in `allowedInlineFunctions` (manual bypass), or + * 2. Its containing call resolves to a Kotlin `inline` function (compiler inlines the body). + */ + private fun KtLambdaExpression.isInlinedLambda(): Boolean { + val containingCall = findContainingCallExpression() ?: return false + val calleeName = containingCall.calleeExpression?.text + val isAllowlisted = calleeName != null && calleeName in allowedInlineFunctionsSet + val isTypedInline = bindingContext != BindingContext.EMPTY && + ( + containingCall.getResolvedCall(bindingContext) + ?.candidateDescriptor as? FunctionDescriptor + )?.isInline == true + return isAllowlisted || isTypedInline + } + + private fun KtLambdaExpression.findContainingCallExpression(): KtCallExpression? { + // Trailing lambda: KtLambdaExpression -> KtLambdaArgument -> KtCallExpression + val viaTrailing = parent?.parent as? KtCallExpression + if (viaTrailing != null) return viaTrailing + // In-parens lambda: KtLambdaExpression -> KtValueArgument -> KtValueArgumentList -> KtCallExpression + val viaParens = parent?.parent?.parent as? KtCallExpression + return viaParens + } + + override fun visitStringTemplateExpression(expression: KtStringTemplateExpression) { + super.visitStringTemplateExpression(expression) + if (!insideHotMethod || isExcluded(HotMethod.CHECK_STRING_TEMPLATE)) return + val hasInterpolation = expression.entries.any { it !is KtLiteralStringTemplateEntry } + if (!hasInterpolation) return + report( + CodeSmell( + issue, + Entity.from(expression), + "String template with interpolation inside a @HotMethod allocates a `StringBuilder` on every " + + "invocation. Pre-compute the string outside the hot path or use a pre-allocated buffer." + ) + ) + } + + override fun visitResolvedFunctionCall( + expression: KtCallExpression, + resolvedCall: ResolvedFunCall + ) { + if (!insideHotMethod) return + + val containerFqName = resolvedCall.containerFqName + val functionName = resolvedCall.functionName + + if (matchesForbiddenEntry(containerFqName, functionName, forbiddenFactoryFunctionsSet) && + !isExcluded(HotMethod.CHECK_FACTORY, functionName, "$containerFqName.$functionName") + ) { + report( + CodeSmell( + issue, + Entity.from(expression), + "`$containerFqName.$functionName()` allocates a new collection or builder on every " + + "invocation. Inside a @HotMethod this causes GC pressure. " + + "Pre-allocate outside the hot path and reuse the instance." + ) + ) + return + } + + if (matchesForbiddenEntry(containerFqName, functionName, forbiddenCallsSet) && + !isExcluded(HotMethod.CHECK_COLLECTION_OPS, functionName, "$containerFqName.$functionName") + ) { + report( + CodeSmell( + issue, + Entity.from(expression), + "`$containerFqName.$functionName()` is an O(N) operation. Inside a @HotMethod it runs on every " + + "frame or touch event. Use an index-based for-loop or pre-compute outside the hot path." + ) + ) + } + } + + /** + * Matches `containerFqName.functionName` against an entry from the forbidden set. + * + * An entry can be: + * - `forEach` — method name only, matches any receiver + * - `List.forEach` — matches any container whose FQN ends with `List` + * - `kotlin.collections.List.forEach` — exact FQN match + */ + private fun matchesForbiddenEntry( + containerFqName: String, + functionName: String, + entries: Set + ): Boolean { + val qualifiedName = "$containerFqName.$functionName" + return entries.any { entry -> + if ('.' !in entry) { + functionName == entry + } else { + qualifiedName == entry || qualifiedName.endsWith(".$entry") + } + } + } + + private companion object { + private const val HOT_METHOD_ANNOTATION = "HotMethod" + } +} diff --git a/tools/detekt/src/test/kotlin/com/datadog/tools/detekt/rules/sdk/HotMethodIllegalCallTest.kt b/tools/detekt/src/test/kotlin/com/datadog/tools/detekt/rules/sdk/HotMethodIllegalCallTest.kt new file mode 100644 index 0000000000..523e2028a1 --- /dev/null +++ b/tools/detekt/src/test/kotlin/com/datadog/tools/detekt/rules/sdk/HotMethodIllegalCallTest.kt @@ -0,0 +1,521 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +package com.datadog.tools.detekt.rules.sdk + +import com.datadog.android.internal.lint.HotMethod +import io.github.detekt.test.utils.KotlinCoreEnvironmentWrapper +import io.github.detekt.test.utils.createEnvironment +import io.gitlab.arturbosch.detekt.test.TestConfig +import io.gitlab.arturbosch.detekt.test.assertThat +import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext +import org.assertj.core.api.Assertions.assertThat as assertJThat +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test + +internal class HotMethodIllegalCallTest { + + private lateinit var kotlinEnv: KotlinCoreEnvironmentWrapper + + @BeforeEach + fun setup() { + kotlinEnv = createEnvironment() + } + + @AfterEach + fun tearDown() { + kotlinEnv.dispose() + } + + // region findings — O(N) linear search + + @Test + fun `M report error W find inside HotMethod`() { + assertCallFindings(1, "list.find { it == \"x\" }", "kotlin.collections.List.find") + } + + @Test + fun `M report error W indexOf inside HotMethod`() { + assertCallFindings(1, "list.indexOf(\"x\")", "kotlin.collections.List.indexOf") + } + + @Test + fun `M report error W indexOfFirst inside HotMethod`() { + assertCallFindings(1, "list.indexOfFirst { it == \"x\" }", "kotlin.collections.List.indexOfFirst") + } + + @Test + fun `M report error W any inside HotMethod`() { + assertCallFindings(1, "list.any { it == \"x\" }", "kotlin.collections.List.any") + } + + @Test + fun `M report error W count inside HotMethod`() { + assertCallFindings(1, "list.count { it == \"x\" }", "kotlin.collections.List.count") + } + + // endregion + + // region findings — O(N) full iteration + + @Test + fun `M report error W forEach inside HotMethod`() { + assertCallFindings(1, "list.forEach { println(it) }", "kotlin.collections.List.forEach") + } + + @Test + fun `M report error W forEachIndexed inside HotMethod`() { + assertCallFindings(1, "list.forEachIndexed { _, v -> println(v) }", "kotlin.collections.List.forEachIndexed") + } + + @Test + fun `M report error W fold inside HotMethod`() { + assertCallFindings(1, "list.fold(\"\") { acc, _ -> acc }", "kotlin.collections.List.fold") + } + + @Test + fun `M report error W reduce inside HotMethod`() { + assertCallFindings(1, "list.reduce { acc, _ -> acc }", "kotlin.collections.List.reduce") + } + + @Test + fun `M report error W sumOf inside HotMethod`() { + assertCallFindings(1, "list.sumOf { it.length }", "kotlin.collections.List.sumOf") + } + + @Test + fun `M report error W maxByOrNull inside HotMethod`() { + assertCallFindings(1, "list.maxByOrNull { it.length }", "kotlin.collections.List.maxByOrNull") + } + + // endregion + + // region findings — materialising + + @Test + fun `M report error W filter inside HotMethod`() { + assertCallFindings(1, "list.filter { it.isNotEmpty() }", "kotlin.collections.List.filter") + } + + @Test + fun `M report error W map inside HotMethod`() { + assertCallFindings(1, "list.map { it.uppercase() }", "kotlin.collections.List.map") + } + + @Test + fun `M report error W flatMap inside HotMethod`() { + assertCallFindings(1, "list.flatMap { emptyList() }", "kotlin.collections.List.flatMap") + } + + @Test + fun `M report error W toList inside HotMethod`() { + assertCallFindings(1, "list.toList()", "kotlin.collections.List.toList") + } + + @Test + fun `M report error W toSet inside HotMethod`() { + assertCallFindings(1, "list.toSet()", "kotlin.collections.List.toSet") + } + + @Test + fun `M report error W groupBy inside HotMethod`() { + assertCallFindings(1, "list.groupBy { it.first() }", "kotlin.collections.List.groupBy") + } + + @Test + fun `M report error W partition inside HotMethod`() { + assertCallFindings(1, "list.partition { it.isNotEmpty() }", "kotlin.collections.List.partition") + } + + // endregion + + // region findings — sorting + + @Test + fun `M report error W sorted inside HotMethod`() { + assertCallFindings(1, "list.sorted()", "kotlin.collections.List.sorted") + } + + @Test + fun `M report error W sortedBy inside HotMethod`() { + assertCallFindings(1, "list.sortedBy { it.length }", "kotlin.collections.List.sortedBy") + } + + @Test + fun `M report error W sortedWith inside HotMethod`() { + assertCallFindings(1, "list.sortedWith(compareBy { it })", "kotlin.collections.List.sortedWith") + } + + // endregion + + // region findings — factory functions + + @Test + fun `M report error W mutableListOf inside HotMethod`() { + assertFactoryFindings(1, "val x = mutableListOf()", "kotlin.collections.mutableListOf") + } + + @Test + fun `M report error W listOf inside HotMethod`() { + assertFactoryFindings(1, "val x = listOf()", "kotlin.collections.listOf") + } + + @Test + fun `M report error W mutableSetOf inside HotMethod`() { + assertFactoryFindings(1, "val x = mutableSetOf()", "kotlin.collections.mutableSetOf") + } + + @Test + fun `M report error W mutableMapOf inside HotMethod`() { + assertFactoryFindings(1, "val x = mutableMapOf()", "kotlin.collections.mutableMapOf") + } + + @Test + fun `M report error W hashMapOf inside HotMethod`() { + assertFactoryFindings(1, "val x = hashMapOf()", "kotlin.collections.hashMapOf") + } + + @Test + fun `M report error W buildString inside HotMethod`() { + assertFactoryFindings(1, "val x = buildString { append(\"x\") }", "kotlin.text.buildString") + } + + @Test + fun `M report error W buildList inside HotMethod`() { + assertFactoryFindings(1, "val x = buildList { add(\"x\") }", "kotlin.collections.buildList") + } + + @Test + fun `M report error W buildMap inside HotMethod`() { + assertFactoryFindings(1, "val x = buildMap { put(\"x\", 1) }", "kotlin.collections.buildMap") + } + + // endregion + + // region findings — lambda and string allocation + + @Test + fun `M report error W lambda literal inside HotMethod`() { + val code = hotMethodCode("val f: () -> Unit = { println(\"x\") }") + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).hasSize(1) + } + + @Test + fun `M report error W trailing lambda to non-inline function inside HotMethod`() { + val code = """ + annotation class HotMethod(val message: String) + fun nonInlineReceiver(block: () -> Unit) { block() } + class Foo { + @HotMethod(message = "hot") + fun hotFoo() { + nonInlineReceiver { println("x") } + } + } + """.trimIndent() + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).hasSize(1) + } + + @Test + fun `M not report W lambda passed to allowedInlineFunction`() { + val code = hotMethodCode("val x = \"hello\".let { it.length }") + val rule = HotMethodIllegalCall(TestConfig("allowedInlineFunctions" to listOf("let"))) + assertThat(rule.compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M not report W lambda outside HotMethod`() { + val code = hotMethodCode("val f: () -> Unit = { println(\"x\") }", annotated = false) + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M report error W string template with interpolation inside HotMethod`() { + val code = hotMethodCode("val tag = \"value=\$list\"") + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).hasSize(1) + } + + @Test + fun `M not report W plain string literal inside HotMethod`() { + val code = hotMethodCode("val tag = \"plain literal\"") + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M not report W string template with interpolation outside HotMethod`() { + val code = hotMethodCode("val tag = \"value=\$list\"", annotated = false) + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + // endregion + + // region findings — object allocation + + @Test + fun `M report error W constructor call inside HotMethod`() { + val code = hotMethodCode("val sb = StringBuilder()") + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).hasSize(1) + } + + @Test + fun `M report error W anonymous object inside HotMethod`() { + val code = """ + annotation class HotMethod(val message: String) + interface Task { fun run() } + class Foo { + @HotMethod(message = "hot") + fun hotFoo() { + val t = object : Task { override fun run() {} } + } + } + """.trimIndent() + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).hasSize(1) + } + + // endregion + + // region class-qualified precision — custom class with same name must NOT be flagged + + @Test + fun `M not report O(N) W custom class method with same name as forbidden call`() { + // The O(N) checks are class-qualified and must NOT match MyDomainQuery. + // However, lambda arguments to non-inline functions DO still allocate and ARE flagged. + val code = """ + annotation class HotMethod(val message: String) + + class MyDomainQuery { + fun findByKey(key: String): String? = null + fun processAll() {} + } + + class Foo(private val query: MyDomainQuery) { + @HotMethod(message = "hot") + fun hotFoo() { + query.findByKey("x") + query.processAll() + } + } + """.trimIndent() + val rule = HotMethodIllegalCall( + TestConfig( + "forbiddenCalls" to listOf( + "kotlin.collections.List.find", + "kotlin.collections.List.forEach", + "kotlin.collections.List.filter" + ) + ) + ) + assertThat(rule.compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + // endregion + + // region no findings — context and scope + + @Test + fun `M not report W forbidden call outside HotMethod`() { + val code = hotMethodCode("list.find { it == \"x\" }", annotated = false) + val rule = HotMethodIllegalCall(TestConfig("forbiddenCalls" to listOf("kotlin.collections.List.find"))) + assertThat(rule.compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M not report W index-based for-loop inside HotMethod`() { + val code = hotMethodCode("for (i in list.indices) println(list[i])") + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M not report W local named function inside HotMethod uses forbidden call`() { + val code = hotMethodCode("fun localHelper() { list.find { it == \"x\" } }\nlocalHelper()") + val rule = HotMethodIllegalCall( + TestConfig( + "forbiddenCalls" to listOf("kotlin.collections.List.find"), + "allowedInlineFunctions" to listOf("find") + ) + ) + assertThat(rule.compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M not report W constructor call outside HotMethod`() { + val code = hotMethodCode("val sb = StringBuilder()", annotated = false) + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + // endregion + + // region exclude parameter + + @Test + fun `M not report constructor W exclude contains constructor category`() { + val code = hotMethodCode("val sb = StringBuilder()", exclude = listOf(HotMethod.CHECK_CONSTRUCTOR)) + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M not report constructor W exclude contains specific constructor name`() { + val code = hotMethodCode("val sb = StringBuilder()", exclude = listOf("StringBuilder")) + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M still report other constructor W exclude contains only unrelated name`() { + val code = hotMethodCode("val sb = StringBuilder()", exclude = listOf("ArrayList")) + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).hasSize(1) + } + + @Test + fun `M not report lambda W exclude contains lambda category`() { + val code = hotMethodCode("val f: () -> Unit = {}", exclude = listOf(HotMethod.CHECK_LAMBDA)) + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M not report string template W exclude contains string-template category`() { + val code = hotMethodCode("val s = \"value=\${list.size}\"", exclude = listOf(HotMethod.CHECK_STRING_TEMPLATE)) + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M not report forEach W exclude contains specific function name`() { + val code = hotMethodCode("list.forEach { println(it) }", exclude = listOf("forEach")) + val rule = HotMethodIllegalCall( + TestConfig( + "forbiddenCalls" to listOf("kotlin.collections.List.forEach"), + "allowedInlineFunctions" to listOf("forEach") + ) + ) + assertThat(rule.compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M not report forEach W exclude contains qualified name`() { + val code = hotMethodCode("list.forEach { println(it) }", exclude = listOf("kotlin.collections.List.forEach")) + val rule = HotMethodIllegalCall( + TestConfig( + "forbiddenCalls" to listOf("kotlin.collections.List.forEach"), + "allowedInlineFunctions" to listOf("forEach") + ) + ) + assertThat(rule.compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M not report collection-ops W exclude contains collection-ops category`() { + val code = hotMethodCode("list.forEach { println(it) }", exclude = listOf(HotMethod.CHECK_COLLECTION_OPS)) + val rule = HotMethodIllegalCall( + TestConfig( + "forbiddenCalls" to listOf("kotlin.collections.List.forEach"), + "allowedInlineFunctions" to listOf("forEach") + ) + ) + assertThat(rule.compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M not report anonymous object W exclude contains anonymous-object category`() { + val code = """ + annotation class HotMethod(val message: String, val exclude: Array = []) + interface Task { fun run() } + class Foo { + @HotMethod(message = "hot", exclude = ["${HotMethod.CHECK_ANONYMOUS_OBJECT}"]) + fun hotFoo() { + val t = object : Task { override fun run() {} } + } + } + """.trimIndent() + assertThat(HotMethodIllegalCall().compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M not report factory function W exclude contains factory category`() { + val code = hotMethodCode("val x = mutableListOf()", exclude = listOf(HotMethod.CHECK_FACTORY)) + val rule = HotMethodIllegalCall( + TestConfig("forbiddenFactoryFunctions" to listOf("kotlin.collections.mutableListOf")) + ) + assertThat(rule.compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M not report factory function W exclude contains specific factory name`() { + val code = hotMethodCode("val x = mutableListOf()", exclude = listOf("mutableListOf")) + val rule = HotMethodIllegalCall( + TestConfig("forbiddenFactoryFunctions" to listOf("kotlin.collections.mutableListOf")) + ) + assertThat(rule.compileAndLintWithContext(kotlinEnv.env, code)).isEmpty() + } + + @Test + fun `M still report other calls W exclude only silences specific name`() { + val code = hotMethodCode("list.find { it == \"x\" }\nlist.forEach { println(it) }", exclude = listOf("find")) + val rule = HotMethodIllegalCall( + TestConfig( + "forbiddenCalls" to listOf("kotlin.collections.List.find", "kotlin.collections.List.forEach"), + "allowedInlineFunctions" to listOf("find", "forEach") + ) + ) + // find is excluded, forEach is not → exactly 1 finding + val findings = rule.compileAndLintWithContext(kotlinEnv.env, code) + assertThat(findings).hasSize(1) + assertJThat(findings.first().message).contains("forEach") + } + + // endregion + + // region helpers + + private fun assertCallFindings(expected: Int, callCode: String, forbiddenCall: String) { + // The stdlib functions in forbiddenCalls are inline — their lambdas do not allocate. + // Add the method name to allowedInlineFunctions so the lambda is not also flagged. + val method = forbiddenCall.substringAfterLast('.') + val rule = HotMethodIllegalCall( + TestConfig( + "forbiddenCalls" to listOf(forbiddenCall), + "allowedInlineFunctions" to listOf(method) + ) + ) + val findings = rule.compileAndLintWithContext(kotlinEnv.env, hotMethodCode(callCode)) + assertThat(findings).hasSize(expected) + } + + private fun assertFactoryFindings(expected: Int, callCode: String, forbiddenFactory: String) { + // Factory functions like buildString/buildList are inline — their lambdas do not allocate. + val method = forbiddenFactory.substringAfterLast('.') + val rule = HotMethodIllegalCall( + TestConfig( + "forbiddenFactoryFunctions" to listOf(forbiddenFactory), + "allowedInlineFunctions" to listOf(method) + ) + ) + val findings = rule.compileAndLintWithContext(kotlinEnv.env, hotMethodCode(callCode)) + assertThat(findings).hasSize(expected) + } + + private fun hotMethodCode( + callCode: String, + annotated: Boolean = true, + exclude: List = emptyList() + ): String { + val excludeParam = if (exclude.isNotEmpty()) { + ", exclude = [${exclude.joinToString(", ") { "\"$it\"" }}]" + } else { + "" + } + val annotation = if (annotated) "@HotMethod(message = \"hot\"$excludeParam)" else "" + return """ + annotation class HotMethod(val message: String, val exclude: Array = []) + class Foo(private val list: List) { + $annotation + fun hotFoo() { + $callCode + } + } + """.trimIndent() + } + + // endregion +}