Skip to content

Commit af9aead

Browse files
committed
RUM-16478: Improving performance
1 parent 088ce74 commit af9aead

5 files changed

Lines changed: 64 additions & 30 deletions

File tree

tools/detekt/src/main/kotlin/com/datadog/tools/detekt/rules/sdk/UnsafeThirdPartyFunctionCall.kt

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,20 @@ class UnsafeThirdPartyFunctionCall(
4141

4242
private val codeParser = CodeParser()
4343
private val ruleParser = DetektConfigParser()
44-
private val configValidator = DetektConfigValidator(ruleParser)
44+
private val configValidator = DetektConfigValidator()
4545
private val internalPackagePrefix: String by config(defaultValue = "")
4646
private val treatUnknownFunctionAsThrowing: Boolean by config(defaultValue = true)
4747
private val knownSafeCalls: List<SignatureRule> by config(emptyList(), ruleParser::parseSafeCalls)
4848
private val knownThrowingCalls: List<SignatureRule> by config(emptyList(), ruleParser::parseThrowingCalls)
4949
private val caughtExceptions = Stack<Set<String>>()
5050

51+
// Exact entries are looked up via O(1) map; wildcards fall through to list scan.
52+
// Throwing category is always checked before safe category (preserves original semantics).
53+
private val exactThrowingCalls: Map<String, SignatureRule>
54+
private val wildcardThrowingCalls: List<SignatureRule>
55+
private val exactSafeCalls: Map<String, SignatureRule>
56+
private val wildcardSafeCalls: List<SignatureRule>
57+
5158
override val issue: Issue = Issue(
5259
javaClass.simpleName,
5360
Severity.Defect,
@@ -58,6 +65,10 @@ class UnsafeThirdPartyFunctionCall(
5865

5966
init {
6067
configValidator.validate(knownSafeCalls, knownThrowingCalls)
68+
exactThrowingCalls = knownThrowingCalls.filterNot { it.hasWildcards }.associateBy { it.source }
69+
wildcardThrowingCalls = knownThrowingCalls.filter { it.hasWildcards }
70+
exactSafeCalls = knownSafeCalls.filterNot { it.hasWildcards }.associateBy { it.source }
71+
wildcardSafeCalls = knownSafeCalls.filter { it.hasWildcards }
6172
}
6273

6374
override fun visitTryExpression(expression: KtTryExpression) {
@@ -77,26 +88,25 @@ class UnsafeThirdPartyFunctionCall(
7788
return
7889
}
7990

80-
classifyAndReport(
81-
expression,
82-
signature = resolvedCall.call,
83-
params = codeParser.parseFormalParams(expression, bindingContext)
84-
)
91+
val params = codeParser.parseFormalParams(expression, bindingContext)
92+
classifyAndReport(expression, signature = resolvedCall.call, params = params)
8593
}
8694

8795
private fun classifyAndReport(expression: KtCallExpression, signature: String, params: List<KtMethodParameter>) {
88-
knownThrowingCalls.firstOrNull { it.matches(signature) }
89-
?.let { throwingMatch ->
90-
throwingMatch.validate(params)
91-
checkCallThrowingExceptions(expression, signature, throwingMatch.exceptions)
92-
return
93-
}
94-
95-
knownSafeCalls.firstOrNull { it.matches(signature) }
96-
?.let { safeMatch ->
97-
safeMatch.validate(params)
98-
return
99-
}
96+
val throwingMatch = exactThrowingCalls[signature]
97+
?: wildcardThrowingCalls.firstOrNull { it.matches(signature) }
98+
if (throwingMatch != null) {
99+
throwingMatch.validate(params)
100+
checkCallThrowingExceptions(expression, signature, throwingMatch.exceptions)
101+
return
102+
}
103+
104+
val safeMatch = exactSafeCalls[signature]
105+
?: wildcardSafeCalls.firstOrNull { it.matches(signature) }
106+
if (safeMatch != null) {
107+
safeMatch.validate(params)
108+
return
109+
}
100110

101111
if (treatUnknownFunctionAsThrowing) {
102112
reportUnsafeCall(

tools/detekt/src/main/kotlin/com/datadog/tools/detekt/rules/sdk/rule/thirdparty/DetektConfigValidator.kt

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@ package com.datadog.tools.detekt.rules.sdk.rule.thirdparty
99
/**
1010
* Validates parsed UnsafeThirdPartyFunctionCall config entries.
1111
*/
12-
internal class DetektConfigValidator(
13-
private val ruleParser: DetektConfigParser = DetektConfigParser()
14-
) {
12+
internal class DetektConfigValidator {
1513

1614
fun validate(
1715
knownSafeCalls: List<SignatureRule>,
@@ -36,8 +34,9 @@ internal class DetektConfigValidator(
3634
safePatterns: List<SignatureRule>,
3735
throwingPatterns: List<SignatureRule>
3836
) = buildList {
37+
val throwingByKey = throwingPatterns.groupBy { it.overlapKey }
3938
safePatterns.forEach { safe ->
40-
throwingPatterns.forEach { throwing ->
39+
throwingByKey[safe.overlapKey].orEmpty().forEach { throwing ->
4140
if (safe.intersects(throwing)) {
4241
add(
4342
" - '${safe.source}' (knownSafeCalls) overlaps with " +
@@ -49,9 +48,9 @@ internal class DetektConfigValidator(
4948
}
5049

5150
private fun validateThrowingCallIsNotInSafeCallsConfig(safePatterns: List<SignatureRule>) = buildList {
52-
val semanticUnsafePatterns = ruleParser.parseThrowingCalls(semanticUnsafeCalls)
51+
val semanticUnsafeByKey = semanticUnsafePatterns.groupBy { it.overlapKey }
5352
safePatterns.forEach { safe ->
54-
semanticUnsafePatterns.forEach { unsafe ->
53+
semanticUnsafeByKey[safe.overlapKey].orEmpty().forEach { unsafe ->
5554
if (safe.intersects(unsafe)) {
5655
add(
5756
" - '${safe.source}' (knownSafeCalls) overlaps with known unsafe " +
@@ -66,10 +65,15 @@ internal class DetektConfigValidator(
6665
configName: String,
6766
patterns: List<SignatureRule>
6867
) = buildList {
69-
for (i in patterns.indices) {
70-
for (j in i + 1 until patterns.size) {
71-
if (patterns[i].intersects(patterns[j])) {
72-
add(" - '${patterns[i].source}' duplicates '${patterns[j].source}' in $configName")
68+
patterns.groupBy { it.overlapKey }.values.forEach { matchingPatterns ->
69+
for (i in matchingPatterns.indices) {
70+
for (j in i + 1 until matchingPatterns.size) {
71+
if (matchingPatterns[i].intersects(matchingPatterns[j])) {
72+
add(
73+
" - '${matchingPatterns[i].source}' duplicates " +
74+
"'${matchingPatterns[j].source}' in $configName"
75+
)
76+
}
7377
}
7478
}
7579
}
@@ -80,5 +84,7 @@ internal class DetektConfigValidator(
8084
"java.lang.Thread.interrupt():java.lang.SecurityException",
8185
"kotlin.Array.first(kotlin.Function1):java.util.NoSuchElementException"
8286
)
87+
88+
private val semanticUnsafePatterns = DetektConfigParser().parseThrowingCalls(semanticUnsafeCalls)
8389
}
8490
}

tools/detekt/src/main/kotlin/com/datadog/tools/detekt/rules/sdk/rule/thirdparty/SignatureRule.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ internal class SignatureRule(
2424
) {
2525

2626
private val methodPart: String = source.substringBefore('(')
27+
internal val overlapKey: OverlapKey = OverlapKey(methodPart, paramSlots.size)
28+
29+
/** True when this rule has no wildcard slots and can be matched with a plain map lookup. */
30+
internal val hasWildcards: Boolean = wildcardSlots.isNotEmpty()
2731

2832
/** Returns true if this rule's pattern matches the given fully-qualified [signature] string. */
2933
fun matches(signature: String): Boolean =
@@ -59,6 +63,8 @@ internal class SignatureRule(
5963

6064
internal data class WildcardSlot(val index: Int, val symbol: String)
6165

66+
internal data class OverlapKey(val methodPart: String, val paramCount: Int)
67+
6268
private companion object {
6369

6470
private fun slotsMatches(a: String, b: String): Boolean {

tools/detekt/src/test/kotlin/com/datadog/tools/detekt/rules/sdk/UnsafeThirdPartyFunctionCallTest.kt

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,19 @@ internal class UnsafeThirdPartyFunctionCallTest {
7979
assertThat(findings).hasSize(1)
8080
}
8181

82+
@Test
83+
fun `M throw exception W init {invalid detekt config}`() {
84+
// Given
85+
val config = TestConfig(
86+
"knownSafeCalls" to listOf("kotlin.Array.first(kotlin.Function1)")
87+
)
88+
89+
// When / Then
90+
assertThrows<IllegalStateException> {
91+
UnsafeThirdPartyFunctionCall(config)
92+
}
93+
}
94+
8295
@Test
8396
fun `ignore call on internal package extension function`() {
8497
// Given
@@ -777,5 +790,4 @@ internal class UnsafeThirdPartyFunctionCallTest {
777790
}
778791

779792
// endregion
780-
781793
}

tools/detekt/src/test/kotlin/com/datadog/tools/detekt/rules/sdk/rule/thirdparty/DetektConfigValidatorTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import org.junit.jupiter.api.assertThrows
1212
internal class DetektConfigValidatorTest {
1313

1414
private val parser = DetektConfigParser()
15-
private val testedValidator = DetektConfigValidator(parser)
15+
private val testedValidator = DetektConfigValidator()
1616

1717
@Test
1818
fun `M throw exception W validate() {known unsafe literal is listed as safe}`() {

0 commit comments

Comments
 (0)