chore: add no-unused-expressions lint rule #7007

simon-id · 2025-11-27T13:30:08Z

What does this PR do?

Add the eslint rule no-unused-expressions, it prevents stuff like:

function a () {
  if (x) false // forgot the return statement

  blabla()
}

It also allows the use of the void operator, to explicitly allow statements with only side effects.

Motivation

Mistakes have been made in previous PRs that could have been prevented with this rule.

github-actions · 2025-11-27T13:30:27Z

Overall package size

Self size: 13.41 MB
Deduped: 113.61 MB
No deduping: 128.63 MB

Dependency sizes

| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.7.0 | 35.02 MB | 35.02 MB | | @datadog/native-appsec | 10.3.0 | 20.73 MB | 20.74 MB | | @datadog/pprof | 5.12.0 | 11.19 MB | 11.57 MB | | @datadog/native-iast-taint-tracking | 4.1.0 | 9.01 MB | 9.02 MB | | @opentelemetry/resources | 1.30.1 | 557.67 kB | 7.71 MB | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.5.4 | 2.95 MB | 5.83 MB | | @datadog/wasm-js-rewriter | 5.0.1 | 2.82 MB | 3.53 MB | | @datadog/native-metrics | 3.1.1 | 1.02 MB | 1.43 MB | | @opentelemetry/api-logs | 0.208.0 | 199.48 kB | 1.42 MB | | @opentelemetry/api | 1.9.0 | 1.22 MB | 1.22 MB | | jsonpath-plus | 10.3.0 | 617.18 kB | 1.08 MB | | import-in-the-middle | 1.15.0 | 127.66 kB | 856.24 kB | | lru-cache | 10.4.3 | 804.3 kB | 804.3 kB | | @datadog/openfeature-node-server | 0.2.0 | 118.51 kB | 437.19 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | source-map | 0.7.6 | 185.63 kB | 185.63 kB | | pprof-format | 2.2.1 | 163.06 kB | 163.06 kB | | @datadog/sketches-js | 2.1.1 | 109.9 kB | 109.9 kB | | @isaacs/ttlcache | 2.1.2 | 90.79 kB | 90.79 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 7.0.5 | 63.38 kB | 63.38 kB | | istanbul-lib-coverage | 3.2.2 | 34.37 kB | 34.37 kB | | rfdc | 1.4.1 | 27.15 kB | 27.15 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | shell-quote | 1.8.3 | 23.74 kB | 23.74 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | semifies | 1.0.0 | 15.84 kB | 15.84 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | mutexify | 1.4.0 | 5.71 kB | 8.74 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | module-details-from-path | 1.0.4 | 3.96 kB | 3.96 kB | | escape-string-regexp | 5.0.0 | 3.66 kB | 3.66 kB |

_{🤖 This report was automatically generated by heaviest-objects-in-the-universe}

codecov · 2025-11-27T13:30:48Z

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 84.94%. Comparing base (0bb1f17) to head (61593c8).

Files with missing lines	Patch %	Lines
packages/datadog-instrumentations/src/dns.js	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7007   +/-   ##
=======================================
  Coverage   84.94%   84.94%           
=======================================
  Files         514      514           
  Lines       21754    21754           
=======================================
  Hits        18478    18478           
  Misses       3276     3276

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

pr-commenter · 2025-11-27T13:39:13Z

Benchmarks

Benchmark execution time: 2025-12-03 09:41:47

Comparing candidate commit 61593c8 in PR branch simon-id-patch-2 with baseline commit 0bb1f17 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 289 metrics, 31 unstable metrics.

simon-id · 2025-12-02T12:42:21Z

packages/datadog-instrumentations/src/mariadb.js

          return result
        }, error => {
-          ctx.error
+          ctx.error = error


@rochdev need your approval on this, the change i made is only a guess

This looks like a bug fix. Could you please add a test for that?

yes, but i'll let roch do this, because i don't even know what bug this fixes

Well, that is to figure out in that case, no?

Because it would either not be needed, or it would fix a bug and we should add a regression test.
Not adding the test is not good in my opinion.

Yeah of course I agree, I'd like a test for this in this PR too. I'll try to steal some of Roch's time for this.

BridgeAR · 2025-12-02T17:02:30Z

eslint.config.mjs

      'no-implicit-coercion': ['error', { boolean: true, number: true, string: true, allow: ['!!'] }],
+      'no-unused-expressions': 'error',
      'no-useless-assignment': 'error',
+      'no-void': 'off',


I think it is better to use the eslint exception for accessing the stack, while I am uncertain if accessing it is still needed at all. There were some V8 changes that might make that obsolete (I did not check).

any reasoning against void ? just picked that because i never remember the eslint exception syntax, void is easy to remember

I don't think void is ergonomic as it is unclear why that is needed reading the code. Most developers would also not know what it does (in general), while // eslint-disable-next-line foobar is commonly used and we need that anyway in a few spots.

Ok sounds reasonable, i'll change that

BridgeAR · 2025-12-02T17:04:08Z

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

      return tracer.extract('http_headers', Object.fromEntries(ctx.httpRequest.headers))
    default:
-      null
+      return null


AFAIK it's even possible to just remove the default which would allow this to be a simple if statement. While being on this: the String() call is not needed.

Of course the default here is not needed if the caller accept undefined instead of null, but i'm trying to keep the changes as iso as possible, this is just a linter PR, if i stop at every little thing that can be improved, the PR will be very off topic

I thought it was unnecessary when i saw it too, but if i go and change it, i run the risk of creating more problems and failing tests and stuff, that i'm not interested in investing time on now. Especially since I'm making this PR completely online without running it locally, so iteration time is way slower. Small PR for a small task

Since it didn't return null before, but undefined, I'd argue that there's a bigger risk making this change vs just removing the entire default statement. If you're unsure, look at how the caller handles the return value.

Very good point, you convinced me, I'll make it a if and ping whoever wrote this

Generally, it is good to check where it is used and in what context. The call site uses null next to it working with undefined in that case.
Interestingly, it does have a different behavior using null or undefined looking at the code that uses it. It does seem undefined would be expected in all cases, as far as I can tell by briefly looking at it. The behavior in that case is that the async local storage store is checked if there is no child and that span is then defined as child span in case it exists. The type definition for startSpan childOf option officially only supports undefined, so that is another indicator for that.

Could you please add a TODO entry for the null part and ask someone who maintains this code to look into the null case? I could imagine it is a bug.

BridgeAR · 2025-12-02T17:04:34Z

packages/datadog-instrumentations/src/mariadb.js

          return result
        }, error => {
-          ctx.error
+          ctx.error = error


This looks like a bug fix. Could you please add a test for that?

packages/dd-trace/src/remote_config/scheduler.js

watson · 2025-12-03T09:11:13Z

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

      return tracer.extract('http_headers', Object.fromEntries(ctx.httpRequest.headers))
    default:
-      null
+      return null


Since it didn't return null before, but undefined, I'd argue that there's a bigger risk making this change vs just removing the entire default statement. If you're unsure, look at how the caller handles the return value.

Co-authored-by: Thomas Watson <[email protected]>

Update eslint.config.mjs

aede19c

zeui

42e153b

simon-id added the semver-patch label Dec 1, 2025

simon-id added 5 commits December 1, 2025 17:26

add void before error.stack

1ec0138

replace short-circuit with optional chaining

41f60e0

replace short-circuit with if condition

4586202

add probably missing return

c851f95

order rule correctly

7ab17ec

simon-id changed the title ~~Update eslint.config.mjs~~ chore: add no-unused-expressions lint rule Dec 1, 2025

Merge branch 'master' into simon-id-patch-2

2182259

simon-id self-assigned this Dec 1, 2025

This comment has been minimized.

Sign in to view

simon-id added 2 commits December 1, 2025 19:31

disable no-void

511df85

fix weird line

5782f36

simon-id marked this pull request as ready for review December 2, 2025 12:41

simon-id requested review from a team as code owners December 2, 2025 12:41

simon-id requested review from khanayan123 and removed request for a team December 2, 2025 12:41

simon-id commented Dec 2, 2025

View reviewed changes

BridgeAR reviewed Dec 2, 2025

View reviewed changes

rochdev previously approved these changes Dec 2, 2025

View reviewed changes

watson reviewed Dec 3, 2025

View reviewed changes

Update packages/dd-trace/src/remote_config/scheduler.js

61593c8

Co-authored-by: Thomas Watson <[email protected]>

simon-id dismissed rochdev’s stale review via 61593c8 December 3, 2025 09:30

chore: add no-unused-expressions lint rule #7007

Are you sure you want to change the base?

chore: add no-unused-expressions lint rule #7007

Conversation

simon-id commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Uh oh!

github-actions bot commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overall package size

Uh oh!

codecov bot commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pr-commenter bot commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Uh oh!

This comment has been minimized.

simon-id Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

simon-id commented Nov 27, 2025 •

edited

Loading

github-actions bot commented Nov 27, 2025 •

edited

Loading

codecov bot commented Nov 27, 2025 •

edited

Loading

pr-commenter bot commented Nov 27, 2025 •

edited

Loading

simon-id Dec 2, 2025 •

edited

Loading