Skip to content

Commit 20b66c2

Browse files
BridgeARpabloerhard
authored andcommitted
refactor: stop probing emptiness with Object.keys(obj).length (#9548)
`Object.keys(obj).length` allocates a full key array when callers only need to know whether data exists. Replace those probes with producer-owned presence state, `undefined` for absent results, and existing API absence signals, preserving the distinction between absent and populated data without a consumer-side allocation. A narrow `no-restricted-syntax` rule catches the pattern only in boolean/test positions, leaving genuine counts valid. Explicit exceptions remain for arbitrary user-defined keys and copy paths where separate presence tracking would change existing semantics.
1 parent 86e0817 commit 20b66c2

31 files changed

Lines changed: 290 additions & 132 deletions

File tree

eslint.config.mjs

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,37 @@ const GLOBAL_RESTRICTED_REQUIRES = [
107107
},
108108
]
109109

110+
const SRC_RESTRICTED_SYNTAX = [
111+
{
112+
// Inline `.evaluate(<fn>)` callbacks (Playwright/Puppeteer) are serialized with
113+
// `toString()` and run in chromium — coverage counters inside would ReferenceError.
114+
selector:
115+
"CallExpression[callee.property.name='evaluate']" +
116+
":matches([arguments.0.type='ArrowFunctionExpression'], [arguments.0.type='FunctionExpression'])",
117+
message:
118+
'Move the inline `.evaluate(...)` callback into a `*-browser-scripts.js` file ' +
119+
'(NYC-excluded in nyc.config.js) and import it here.',
120+
},
121+
{
122+
// Static-analysis bundlers (esbuild, webpack, rollup) only see literals as require
123+
// arguments; once any transform (e.g. NYC) wraps them, this shape breaks bundling.
124+
selector: "CallExpression[callee.name='require'][arguments.0.type='ConditionalExpression']",
125+
message: 'Use `cond ? require(\'a\') : require(\'b\')` instead of `require(cond ? \'a\' : \'b\')`.',
126+
},
127+
]
128+
129+
// Matches only probe positions; a genuine count (`writeMapPrefix(Object.keys(x).length)`) must stay allowed.
130+
const OBJECT_KEYS_LENGTH_PROBE = {
131+
selector:
132+
':matches(BinaryExpression[right.value=0], BinaryExpression[left.value=0], UnaryExpression[operator="!"],' +
133+
' IfStatement, ConditionalExpression, LogicalExpression, WhileStatement, DoWhileStatement)' +
134+
" > MemberExpression[property.name='length']" +
135+
" > CallExpression[callee.object.name='Object'][callee.property.name='keys']",
136+
message: 'Do not probe emptiness with `Object.keys(obj).length`; the keys array is allocated on every call. ' +
137+
'Track presence with a boolean at the assignment site, probe a known key (`obj.field !== undefined`), or ' +
138+
'return `undefined` when there is nothing to report instead of an empty object.',
139+
}
140+
110141
export default [
111142
{
112143
name: 'dd-trace/global-ignore',
@@ -643,21 +674,7 @@ export default [
643674
'eslint-rules/eslint-prefer-set-service-name': 'error',
644675
'eslint-rules/eslint-timer-unref': 'error',
645676

646-
'no-restricted-syntax': ['error', {
647-
// Inline `.evaluate(<fn>)` callbacks (Playwright/Puppeteer) are serialized with
648-
// `toString()` and run in chromium — coverage counters inside would ReferenceError.
649-
selector:
650-
"CallExpression[callee.property.name='evaluate']" +
651-
":matches([arguments.0.type='ArrowFunctionExpression'], [arguments.0.type='FunctionExpression'])",
652-
message:
653-
'Move the inline `.evaluate(...)` callback into a `*-browser-scripts.js` file ' +
654-
'(NYC-excluded in nyc.config.js) and import it here.',
655-
}, {
656-
// Static-analysis bundlers (esbuild, webpack, rollup) only see literals as require
657-
// arguments; once any transform (e.g. NYC) wraps them, this shape breaks bundling.
658-
selector: "CallExpression[callee.name='require'][arguments.0.type='ConditionalExpression']",
659-
message: 'Use `cond ? require(\'a\') : require(\'b\')` instead of `require(cond ? \'a\' : \'b\')`.',
660-
}],
677+
'no-restricted-syntax': ['error', ...SRC_RESTRICTED_SYNTAX],
661678

662679
'n/no-restricted-require': ['error', [
663680
...GLOBAL_RESTRICTED_REQUIRES,
@@ -793,6 +810,16 @@ export default [
793810
'unicorn/prefer-optional-catch-binding': 'error',
794811
},
795812
},
813+
{
814+
name: 'dd-trace/packages/src',
815+
files: [
816+
'packages/*/src/**/*.js',
817+
'packages/*/src/**/*.mjs',
818+
],
819+
rules: {
820+
'no-restricted-syntax': ['error', ...SRC_RESTRICTED_SYNTAX, OBJECT_KEYS_LENGTH_PROBE],
821+
},
822+
},
796823
{
797824
name: 'dd-trace/config-sync',
798825
files: [

packages/datadog-instrumentations/src/cucumber.js

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ let pickleByFile = {}
116116
const pickleResultByFile = {}
117117

118118
let skippableSuites = []
119-
let skippableSuitesCoverage = {}
119+
let skippableSuitesCoverage
120120
let skippedSuitesCoverage = {}
121121
let itrCorrelationId = ''
122122
let isForcedToRun = false
@@ -153,12 +153,6 @@ function isValidKnownTests (receivedKnownTests) {
153153
return !!receivedKnownTests.cucumber
154154
}
155155

156-
function hasSkippableSuitesCoverage () {
157-
return skippableSuitesCoverage &&
158-
typeof skippableSuitesCoverage === 'object' &&
159-
Object.keys(skippableSuitesCoverage).length > 0
160-
}
161-
162156
function isTiaCoverageBackfillEnabled () {
163157
return isItrEnabled && isCoverageReportUploadEnabled
164158
}
@@ -172,7 +166,7 @@ function shouldReportCodeCoverageLinesPct (hasBackfilledCoverage) {
172166
}
173167

174168
function getSkippedSuitesCoverageForRun () {
175-
return isSuitesSkipped && isTiaCoverageBackfillEnabled() && hasSkippableSuitesCoverage()
169+
return isSuitesSkipped && isTiaCoverageBackfillEnabled() && skippableSuitesCoverage !== undefined
176170
? skippableSuitesCoverage
177171
: {}
178172
}
@@ -188,7 +182,7 @@ function getCucumberTestSessionCoverageFiles () {
188182

189183
function resetSuiteSkippingRunState () {
190184
skippableSuites = []
191-
skippableSuitesCoverage = {}
185+
skippableSuitesCoverage = undefined
192186
skippedSuitesCoverage = {}
193187
skippedSuites = []
194188
isSuitesSkipped = false
@@ -1143,7 +1137,7 @@ function getWrappedStart (start, frameworkVersion, isParallel = false, isCoordin
11431137

11441138
errorSkippableRequest = skippableResponse.err
11451139
skippableSuites = skippableResponse.skippableSuites ?? []
1146-
skippableSuitesCoverage = skippableResponse.skippableSuitesCoverage ?? {}
1140+
skippableSuitesCoverage = skippableResponse.skippableSuitesCoverage
11471141

11481142
if (!errorSkippableRequest) {
11491143
const filteredPickles = isCoordinator

packages/datadog-instrumentations/src/fastify.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ function wrapHookDone (ctx, request, reply, req, name, doneCallback) {
146146
ctx.error = error
147147
publishError(ctx)
148148

149+
// eslint-disable-next-line no-restricted-syntax -- arbitrary cookie names; publishing {} sets a WAF address
149150
const hasCookies = request.cookies && Object.keys(request.cookies).length > 0
150151

151152
if (cookieParserReadCh.hasSubscribers && hasCookies && !cookiesPublished.has(req)) {
@@ -193,6 +194,7 @@ function preHandler (request, reply, done) {
193194
const res = getRes(reply)
194195
const ctx = { req, res }
195196

197+
// eslint-disable-next-line no-restricted-syntax -- arbitrary body keys; publishing {} sets a WAF address
196198
const hasBody = request.body && Object.keys(request.body).length > 0
197199

198200
// For multipart/form-data, the body is not available until after preValidation hook

packages/datadog-instrumentations/src/jest.js

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ const jestSessionState = (globalThis[JEST_SESSION_STATE] ||= {})
112112
const RETRY_TIMES = Symbol.for('RETRY_TIMES')
113113

114114
let skippableSuites = []
115-
let skippableSuitesCoverage = {}
115+
let skippableSuitesCoverage
116116
let skippedSuitesCoverage = {}
117117
let knownTests = {}
118118
let isCodeCoverageEnabled = false
@@ -133,7 +133,7 @@ let isTestManagementTestsEnabled = false
133133
let testManagementTests = {}
134134
let testManagementAttemptToFixRetries = 0
135135
let isImpactedTestsEnabled = false
136-
let modifiedFiles = {}
136+
let modifiedFiles
137137
let repositoryRoot
138138
let lastCoverageMap
139139
let lastCoverageMapRootDir
@@ -690,8 +690,7 @@ function getWrappedEnvironment (BaseEnvironment, jestVersion) {
690690

691691
if (this.isImpactedTestsEnabled) {
692692
try {
693-
const hasImpactedTests = Object.keys(modifiedFiles).length > 0
694-
this.modifiedFiles = hasImpactedTests ? modifiedFiles : this.testEnvironmentOptions._ddModifiedFiles
693+
this.modifiedFiles = modifiedFiles ?? this.testEnvironmentOptions._ddModifiedFiles
695694
} catch (e) {
696695
log.error('Error parsing impacted tests', e)
697696
this.isImpactedTestsEnabled = false
@@ -2491,12 +2490,6 @@ function getRepositoryRootFromTest (test, fallbackRootDir) {
24912490
return getRepositoryRootFromConfig(test?.context?.config, fallbackRootDir)
24922491
}
24932492

2494-
function hasSkippableSuitesCoverage () {
2495-
return skippableSuitesCoverage &&
2496-
typeof skippableSuitesCoverage === 'object' &&
2497-
Object.keys(skippableSuitesCoverage).length > 0
2498-
}
2499-
25002493
function shouldCollectJestCoverageForTia () {
25012494
return shouldReportJestSuiteCoverageForTia() ||
25022495
(isJestCoverageBackfillSupported && isItrEnabled && isCoverageReportUploadEnabled)
@@ -2607,7 +2600,7 @@ function resetLibraryConfiguration () {
26072600
testManagementTests = {}
26082601
testManagementAttemptToFixRetries = 0
26092602
isImpactedTestsEnabled = false
2610-
modifiedFiles = {}
2603+
modifiedFiles = undefined
26112604
repositoryRoot = undefined
26122605
}
26132606

@@ -2629,13 +2622,14 @@ function applySuiteSkipping (originalTests, rootDir, frameworkVersion) {
26292622

26302623
isSuitesSkipped ||= jestSuitesToRun.suitesToRun.length !== originalTests.length
26312624
numSkippedSuites += jestSuitesToRun.skippedSuites.length
2632-
skippedSuitesCoverage = isSuitesSkipped && isTiaCoverageBackfillEnabled() && hasSkippableSuitesCoverage()
2625+
const hasSkippableSuitesCoverage = skippableSuitesCoverage !== undefined
2626+
skippedSuitesCoverage = isSuitesSkipped && isTiaCoverageBackfillEnabled() && hasSkippableSuitesCoverage
26332627
? skippableSuitesCoverage
26342628
: {}
26352629
coverageBackfillContexts = isSuitesSkipped && isTiaCoverageBackfillEnabled()
26362630
? getTestContexts(originalTests)
26372631
: undefined
2638-
coverageBackfillFiles = isSuitesSkipped && isTiaCoverageBackfillEnabled() && hasSkippableSuitesCoverage()
2632+
coverageBackfillFiles = isSuitesSkipped && isTiaCoverageBackfillEnabled() && hasSkippableSuitesCoverage
26392633
? getCoverageBackfillFiles(skippableSuitesCoverage, repositoryRoot, getTestSuitePath)
26402634
: undefined
26412635

@@ -3024,10 +3018,10 @@ function getCliWrapper (isNewJestVersion) {
30243018
skippableSuitesCoverage: receivedSkippableSuitesCoverage,
30253019
} = skippableSuitesResponse || await getChannelPromise(skippableSuitesCh)
30263020
if (err) {
3027-
skippableSuitesCoverage = {}
3021+
skippableSuitesCoverage = undefined
30283022
} else {
30293023
skippableSuites = receivedSkippableSuites
3030-
skippableSuitesCoverage = receivedSkippableSuitesCoverage || {}
3024+
skippableSuitesCoverage = receivedSkippableSuitesCoverage
30313025
}
30323026
skippedSuitesCoverage = {}
30333027
} catch (err) {

packages/datadog-instrumentations/src/mocha/main.js

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ let suitesToSkip = []
9292
let isSuitesSkipped = false
9393
let areAllSuitesSkipped = false
9494
let skippedSuites = []
95-
let skippableSuitesCoverage = {}
95+
let skippableSuitesCoverage
9696
let skippedSuitesCoverage = {}
9797
let itrCorrelationId = ''
9898
let isForcedToRun = false
@@ -203,12 +203,6 @@ function getFilteredSuites (originalSuites) {
203203
}, { suitesToRun: [], skippedSuites: new Set(), suitesToSkipForRun })
204204
}
205205

206-
function hasSkippableSuitesCoverage () {
207-
return skippableSuitesCoverage &&
208-
typeof skippableSuitesCoverage === 'object' &&
209-
Object.keys(skippableSuitesCoverage).length > 0
210-
}
211-
212206
function isTiaCoverageBackfillEnabled () {
213207
return config.isItrEnabled && config.isCoverageReportUploadEnabled
214208
}
@@ -239,7 +233,7 @@ function shouldReportCodeCoverageLinesPct (hasBackfilledCoverage) {
239233
}
240234

241235
function getSkippedSuitesCoverageForRun () {
242-
return isSuitesSkipped && isTiaCoverageBackfillEnabled() && hasSkippableSuitesCoverage()
236+
return isSuitesSkipped && isTiaCoverageBackfillEnabled() && skippableSuitesCoverage !== undefined
243237
? skippableSuitesCoverage
244238
: {}
245239
}
@@ -257,7 +251,7 @@ function resetSuiteSkippingRunState () {
257251
isSuitesSkipped = false
258252
areAllSuitesSkipped = false
259253
skippedSuites = []
260-
skippableSuitesCoverage = {}
254+
skippableSuitesCoverage = undefined
261255
skippedSuitesCoverage = {}
262256
untestedCoverage = undefined
263257
config.repositoryRoot = undefined
@@ -927,11 +921,11 @@ function getExecutionConfiguration (runner, isParallel, frameworkVersion, onFini
927921
} = response || {}
928922
if (!response || err) {
929923
suitesToSkip = []
930-
skippableSuitesCoverage = {}
924+
skippableSuitesCoverage = undefined
931925
} else {
932926
suitesToSkip = skippableSuites
933927
itrCorrelationId = responseItrCorrelationId
934-
skippableSuitesCoverage = responseSkippableSuitesCoverage || {}
928+
skippableSuitesCoverage = responseSkippableSuitesCoverage
935929
}
936930
if (localSuites) {
937931
suitesToSkip = getSuitesToSkipFromPaths(localSuites)

packages/datadog-instrumentations/src/router.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,7 @@ const visitedParams = new WeakSet()
596596
function wrapHandleRequest (original) {
597597
return function wrappedHandleRequest (...args) {
598598
const req = args[0]
599+
// eslint-disable-next-line no-restricted-syntax -- arbitrary param names; publishing {} sets a WAF address
599600
if (routerParamStartCh.hasSubscribers && !visitedParams.has(req.params) && Object.keys(req.params).length) {
600601
visitedParams.add(req.params)
601602

@@ -635,6 +636,7 @@ function wrapParam (original) {
635636
args[1] = shimmer.wrapFunction(args[1], (originalFn) => {
636637
return function wrappedFn (...fnArgs) {
637638
const req = fnArgs[0]
639+
// eslint-disable-next-line no-restricted-syntax -- arbitrary param names; publishing {} sets a WAF address
638640
if (routerParamStartCh.hasSubscribers && Object.keys(req.params).length && !visitedParams.has(req.params)) {
639641
visitedParams.add(req.params)
640642

packages/datadog-plugin-azure-functions/src/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ function setSpanLinks (triggerType, tracer, span, ctx) {
156156
: triggerMetadata.propertiesArray
157157

158158
const addLinkFromProperties = (props) => {
159-
if (!props || Object.keys(props).length === 0) return
159+
if (!props) return
160160
const spanContext = tracer.extract('text_map', props)
161161
if (spanContext) {
162162
span.addLink({ context: spanContext })

packages/datadog-plugin-cypress/src/cypress-plugin.js

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,7 @@ class CypressPlugin {
477477
testsToSkip = []
478478
skippedTests = []
479479
skippedTestIds = new Set()
480-
skippableTestsCoverage = {}
480+
skippableTestsCoverage
481481
testSessionCoverageMap = createCoverageMap()
482482
hasForcedToRunSuites = false
483483
hasUnskippableSuites = false
@@ -566,7 +566,7 @@ class CypressPlugin {
566566
this.testsToSkip = []
567567
this.skippedTests = []
568568
this.skippedTestIds = new Set()
569-
this.skippableTestsCoverage = {}
569+
this.skippableTestsCoverage = undefined
570570
this.testSessionCoverageMap = createCoverageMap()
571571
this.hasForcedToRunSuites = false
572572
this.hasUnskippableSuites = false
@@ -679,17 +679,6 @@ class CypressPlugin {
679679
return this.repositoryRoot || this.rootDir || process.cwd()
680680
}
681681

682-
/**
683-
* Returns whether the backend supplied skipped-test coverage data.
684-
*
685-
* @returns {boolean}
686-
*/
687-
hasSkippableTestsCoverage () {
688-
return !!(this.skippableTestsCoverage &&
689-
typeof this.skippableTestsCoverage === 'object' &&
690-
Object.keys(this.skippableTestsCoverage).length > 0)
691-
}
692-
693682
/**
694683
* Returns whether skipped test coverage should be backfilled into the session coverage map.
695684
*
@@ -699,7 +688,7 @@ class CypressPlugin {
699688
return this.isItrEnabled &&
700689
this.isCoverageReportUploadEnabled &&
701690
this.isTestsSkipped &&
702-
this.hasSkippableTestsCoverage()
691+
this.skippableTestsCoverage !== undefined
703692
}
704693

705694
/**
@@ -1162,7 +1151,7 @@ class CypressPlugin {
11621151
} else {
11631152
const { skippableTests, correlationId, skippableTestsCoverage } = skippableTestsResponse
11641153
this.testsToSkip = skippableTests || []
1165-
this.skippableTestsCoverage = skippableTestsCoverage || {}
1154+
this.skippableTestsCoverage = skippableTestsCoverage
11661155
this.itrCorrelationId = correlationId
11671156
incrementCountMetric(TELEMETRY_ITR_SKIPPED, { testLevel: 'test' }, this.testsToSkip.length)
11681157
}

packages/datadog-plugin-openai-agents/src/integration.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,7 @@ class OpenAIAgentsIntegration {
498498

499499
this.#tagger.tagTextIO(ddSpan, inputValue, outputValue)
500500

501+
// eslint-disable-next-line no-restricted-syntax -- agents-core builds metadata before the plugin receives it
501502
if (info.metadata && Object.keys(info.metadata).length > 0) {
502503
this.#tagger.tagMetadata(ddSpan, info.metadata)
503504
}

0 commit comments

Comments
 (0)