Skip to content

Commit a0c7d40

Browse files
logaretmclaude
logaretm
and
claude
authored
chore(lint): Rule adjustments and fix warnings (#19612)
This is a follow up PR that cleans up our configuration and reverts the downgrade to warning for some of the rules we use. This brings us to a similar level of coverage with eslint. Some rules have sensitivity issue, especially when it comes to optional chaining and types so we will still have a lot of warnings. ## Summary of Changes ### Config changes (`.oxlintrc.json`) #### Globally disabled (TS files) | Rule | Why | |---|---| | `no-redundant-type-constituents` | Many violations are intentional — AI integration types use `'literal' \| string` for autocomplete hints, and `unknown \| X` patterns are common throughout the codebase. Low bug-catching value. | | `restrict-template-expressions` | 81 violations mostly from OTel span attributes and `unknown` values in template strings. Would require `String()` wrappers everywhere for minimal safety gain — the SDK handles these at runtime. | | `await-thenable` | `await` on non-Promises is valid JS — it's a useful pattern for uniformly handling `T \| Promise<T>` without branching. Not a bug. | | `no-base-to-string` | Set to **warn** (not off). Kept visible since `[object Object]` in strings is a real issue, but not blocking CI while we clean up the 22 remaining source violations. | #### Disabled in tests + dev-packages only | Rule | Why | |---|---| | `no-misused-spread` | Tests intentionally spread class instances to create plain fixture objects. | | `require-array-sort-compare` | Test assertions sorting string arrays — `.sort()` without comparator is fine for strings. | | `no-base-to-string` | Tests don't need strict toString safety. | #### Configured | Rule | Why | |---|---| | `no-unused-vars` | Set to warn with `_` prefix ignore patterns (`argsIgnorePattern`, `varsIgnorePattern`, `caughtErrorsIgnorePattern`). Standard convention — unused catch params/args prefixed with `_` are intentional. | ### Dev-packages config (`dev-packages/.oxlintrc.json`) Added `require-array-sort-compare`, `no-misused-spread`, and `no-base-to-string` as off — these rules aren't worth enforcing in test infrastructure. ### Code fixes | Change | Count | What | |---|---|---| | Removed `\| undefined` from optional params | 19 | `param?: T \| undefined` → `param?: T` — the `?` already implies `undefined` | | Prefixed unused catch params with `_` | 25 | `catch (error)` → `catch (_error)` — follows the `_` convention for intentionally unused variables | | Prefixed unused callback param | 1 | `(error, version)` → `(error, _version)` in `bun/scripts/install-bun.js` | ### Result **373 warnings → 31** (22 of which are the intentional `no-base-to-string` warnings we kept visible). Closes #19718 (added automatically) --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
1 parent cd5030a commit a0c7d40

File tree

65 files changed

+97
-92
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+97
-92
lines changed

‎.oxlintrc.json‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"$schema": "./node_modules/oxlint/configuration_schema.json",
3-
"plugins": ["typescript", "import", "jsdoc", "jest", "vitest"],
3+
"plugins": ["typescript", "import", "jsdoc", "vitest"],
44
"jsPlugins": [
55
{
66
"name": "sdk",
@@ -9,6 +9,11 @@
99
],
1010
"categories": {},
1111
"rules": {
12+
"no-unused-vars": [
13+
"warn",
14+
{ "argsIgnorePattern": "^_", "varsIgnorePattern": "^_", "caughtErrorsIgnorePattern": "^_" }
15+
],
16+
1217
// === Base rules from eslint-config-sdk/base.js ===
1318
"no-console": "error",
1419
"no-alert": "error",
@@ -27,28 +32,18 @@
2732
"import/namespace": "off",
2833
"import/no-unresolved": "off",
2934

30-
// === Jest/Vitest rules ===
31-
"jest/no-focused-tests": "error",
32-
"jest/no-disabled-tests": "error",
33-
3435
// === Rules turned off (not enforced in ESLint or causing false positives) ===
3536
"no-control-regex": "off",
3637
"jsdoc/check-tag-names": "off",
3738
"jsdoc/require-yields": "off",
3839
"no-useless-rename": "off",
3940
"no-constant-binary-expression": "off",
40-
"jest/no-conditional-expect": "off",
41-
"jest/expect-expect": "off",
42-
"jest/no-standalone-expect": "off",
43-
"jest/require-to-throw-message": "off",
44-
"jest/valid-title": "off",
45-
"jest/no-export": "off",
46-
"jest/valid-describe-callback": "off",
4741
"vitest/hoisted-apis-on-top": "off",
4842
"vitest/no-conditional-tests": "off",
4943
"no-unsafe-optional-chaining": "off",
5044
"no-eval": "off",
5145
"no-import-assign": "off",
46+
"typescript/no-duplicate-type-constituents": "off",
5247

5348
// === Custom SDK rules (via JS plugin) ===
5449
"sdk/no-eq-empty": "error"
@@ -61,17 +56,17 @@
6156
"typescript/consistent-type-imports": "error",
6257
"typescript/no-unnecessary-type-assertion": "error",
6358
"typescript/prefer-for-of": "error",
64-
// "typescript/no-floating-promises": ["error", { "ignoreVoid": false }],
59+
"typescript/no-floating-promises": ["error", { "ignoreVoid": true }],
6560
"typescript/no-dynamic-delete": "error",
66-
// "typescript/no-unsafe-member-access": "error",
61+
"typescript/no-unsafe-member-access": "error",
6762
"typescript/unbound-method": "error",
6863
"typescript/no-explicit-any": "error",
6964
"typescript/no-empty-function": "off",
70-
71-
// === FIXME: Rules to turn back as error ===
72-
"typescript/prefer-optional-chain": "warn",
73-
"typescript/no-floating-promises": "warn",
74-
"typescript/no-unsafe-member-access": "warn"
65+
"typescript/prefer-optional-chain": ["error"],
66+
"typescript/no-redundant-type-constituents": "off",
67+
"typescript/restrict-template-expressions": "off",
68+
"typescript/await-thenable": "warn",
69+
"typescript/no-base-to-string": "warn"
7570
}
7671
},
7772
{
@@ -111,7 +106,12 @@
111106
"typescript/no-floating-promises": "off",
112107
"typescript/unbound-method": "off",
113108
"max-lines": "off",
114-
"complexity": "off"
109+
"complexity": "off",
110+
"typescript/prefer-optional-chain": "off",
111+
"typescript/no-misused-spread": "off",
112+
"typescript/require-array-sort-compare": "off",
113+
"typescript/no-base-to-string": "off",
114+
"typescript/await-thenable": "off"
115115
}
116116
},
117117
{

‎dev-packages/.oxlintrc.json‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
"rules": {
55
"typescript/no-explicit-any": "off",
66
"max-lines": "off",
7-
"no-unused-expressions": "off"
7+
"no-unused-expressions": "off",
8+
"typescript/require-array-sort-compare": "off",
9+
"typescript/no-misused-spread": "off",
10+
"typescript/no-base-to-string": "off",
11+
"typescript/await-thenable": "off"
812
}
913
}

‎dev-packages/browser-integration-tests/utils/replayHelpers.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ export function replayEnvelopeIsCompressed(resOrReq: Request | Response): boolea
364364
const lines: boolean[] = envelopeString.split('\n').map(line => {
365365
try {
366366
JSON.parse(line);
367-
} catch (error) {
367+
} catch {
368368
// If we fail to parse a line, we _might_ have found a compressed payload,
369369
// so let's check if this is actually the case.
370370
// This is quite hacky but we can't go through `line` because the prior operations
@@ -394,7 +394,7 @@ export const replayEnvelopeParser = (request: Request | null): unknown[] => {
394394
const lines = envelopeString.split('\n').map(line => {
395395
try {
396396
return JSON.parse(line);
397-
} catch (error) {
397+
} catch {
398398
// If we fail to parse a line, we _might_ have found a compressed payload,
399399
// so let's check if this is actually the case.
400400
// This is quite hacky but we can't go through `line` because the prior operations

‎dev-packages/node-integration-tests/suites/tracing/google-genai/scenario.mjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ async function run() {
102102
},
103103
],
104104
});
105-
} catch (error) {
105+
} catch {
106106
// Expected error
107107
}
108108
});

‎dev-packages/node-integration-tests/suites/tracing/tedious/test.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { afterAll, describe, expect, test } from 'vitest';
22
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
33

4-
// eslint-disable-next-line jest/no-disabled-tests
54
describe.skip('tedious auto instrumentation', { timeout: 75_000 }, () => {
65
afterAll(() => {
76
cleanupChildProcesses();

‎dev-packages/node-overhead-gh-action/index.mjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ async function run() {
157157
body,
158158
});
159159
}
160-
} catch (error) {
160+
} catch {
161161
core.error(
162162
"Error updating comment. This can happen for PR's originating from a fork without write permissions.",
163163
);

‎dev-packages/rollup-utils/plugins/npmPlugins.mjs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,6 @@
77
* Sucrase plugin docs: https://github.com/rollup/plugins/tree/master/packages/sucrase
88
*/
99

10-
import * as fs from 'fs';
11-
import * as path from 'path';
12-
1310
import json from '@rollup/plugin-json';
1411
import replace from '@rollup/plugin-replace';
1512
import cleanup from 'rollup-plugin-cleanup';

‎dev-packages/size-limit-gh-action/index.mjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ async function run() {
171171
body,
172172
});
173173
}
174-
} catch (error) {
174+
} catch {
175175
core.error(
176176
"Error updating comment. This can happen for PR's originating from a fork without write permissions.",
177177
);

‎packages/angular/src/errorhandler.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export interface ErrorHandlerOptions {
2626
function tryToUnwrapZonejsError(error: unknown): unknown | Error {
2727
// TODO: once Angular14 is the minimum requirement ERROR_ORIGINAL_ERROR and
2828
// getOriginalError from error.ts can be used directly.
29-
return error && (error as { ngOriginalError: Error }).ngOriginalError
29+
return (error as { ngOriginalError?: Error })?.ngOriginalError
3030
? (error as { ngOriginalError: Error }).ngOriginalError
3131
: error;
3232
}
@@ -39,6 +39,7 @@ function extractHttpModuleError(error: HttpErrorResponse): string | Error {
3939

4040
// ... or an`ErrorEvent`, which can provide us with the message but no stack...
4141
// guarding `ErrorEvent` against `undefined` as it's not defined in Node environments
42+
// oxlint-disable-next-line typescript/prefer-optional-chain
4243
if (typeof ErrorEvent !== 'undefined' && error.error instanceof ErrorEvent && error.error.message) {
4344
return error.error.message;
4445
}

‎packages/astro/src/server/middleware.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,9 +423,10 @@ function getParametrizedRoute(
423423
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
424424
const routesFromManifest = ctx?.[Symbol.for('context.routes')]?.manifest?.routes;
425425

426-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
426+
// oxlint-disable-next-line typescript/no-unsafe-member-access
427427
const matchedRouteSegmentsFromManifest = routesFromManifest?.find(
428428
(route: { routeData?: { route?: string } }) => route?.routeData?.route === rawRoutePattern,
429+
// oxlint-disable-next-line typescript/no-unsafe-member-access
429430
)?.routeData?.segments;
430431

431432
return (

0 commit comments

Comments
 (0)