From d31adcf57093b88c5a0c656c676c1b4f21b7d93c Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Thu, 24 Jul 2025 17:39:47 +0000 Subject: [PATCH] test(linter): improve sorting diagnostics (#12504) Improve (I think) ordering of diagnostic in test snapshots. 1. Sort by filename first, then position. I think this was probably the original intention. 2. Include start column and end line+column in sort, for more deterministic order. 3. Use `sort_by_cached_key`. `Info::new` creates up to 3 `String` allocations, so preferable to cache the result of the closure, to avoid making those allocations repeatedly. `sort_by_key` will likely call the closure many times for each element. If I've understood right, this reporter is only used in tests, so this PR makes no difference to users. --- apps/oxlint/src/output_formatter/default.rs | 14 +++- ...es__typescript_eslint__test.ts@oxlint.snap | 16 ++-- ...nv_globals_-c .oxlintrc.json .@oxlint.snap | 18 ++--- ..._with_plugin_-c .oxlintrc.json@oxlint.snap | 18 ++--- .../test/__snapshots__/e2e.test.ts.snap | 74 +++++++++---------- 5 files changed, 75 insertions(+), 65 deletions(-) diff --git a/apps/oxlint/src/output_formatter/default.rs b/apps/oxlint/src/output_formatter/default.rs index 551c7c3aece92..e3fbc62ed2e62 100644 --- a/apps/oxlint/src/output_formatter/default.rs +++ b/apps/oxlint/src/output_formatter/default.rs @@ -132,8 +132,18 @@ mod test_implementation { let handler = GraphicalReportHandler::new_themed(GraphicalTheme::none()); let mut output = String::new(); - self.diagnostics.sort_by_key(|diagnostic| Info::new(diagnostic).filename); - self.diagnostics.sort_by_key(|diagnostic| Info::new(diagnostic).start.line); + self.diagnostics.sort_by_cached_key(|diagnostic| { + let info = Info::new(diagnostic); + ( + info.filename, + info.start.line, + info.start.column, + info.end.line, + info.end.column, + info.rule_id, + info.message, + ) + }); for diagnostic in &self.diagnostics { handler.render_report(&mut output, diagnostic.as_ref()).unwrap(); diff --git a/apps/oxlint/src/snapshots/_-c fixtures__typescript_eslint__eslintrc.json fixtures__typescript_eslint__test.ts@oxlint.snap b/apps/oxlint/src/snapshots/_-c fixtures__typescript_eslint__eslintrc.json fixtures__typescript_eslint__test.ts@oxlint.snap index 600ea33bd686b..640314a424cdc 100644 --- a/apps/oxlint/src/snapshots/_-c fixtures__typescript_eslint__eslintrc.json fixtures__typescript_eslint__test.ts@oxlint.snap +++ b/apps/oxlint/src/snapshots/_-c fixtures__typescript_eslint__eslintrc.json fixtures__typescript_eslint__test.ts@oxlint.snap @@ -6,6 +6,14 @@ arguments: -c fixtures/typescript_eslint/eslintrc.json fixtures/typescript_eslin working directory: ---------- + ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/typescript/no-namespace.html\typescript-eslint(no-namespace)]8;;\: ES2015 module syntax is preferred over namespaces. + ,-[fixtures/typescript_eslint/test.ts:1:1] + 1 | namespace X { + : ^^^^^^^^^ + 2 | } + `---- + help: Replace the namespace with an ES2015 module or use `declare module` + ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-unused-vars.html\eslint(no-unused-vars)]8;;\: Variable 'X' is declared but never used. ,-[fixtures/typescript_eslint/test.ts:1:11] 1 | namespace X { @@ -15,14 +23,6 @@ working directory: `---- help: Consider removing this declaration. - ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/typescript/no-namespace.html\typescript-eslint(no-namespace)]8;;\: ES2015 module syntax is preferred over namespaces. - ,-[fixtures/typescript_eslint/test.ts:1:1] - 1 | namespace X { - : ^^^^^^^^^ - 2 | } - `---- - help: Replace the namespace with an ES2015 module or use `declare module` - x ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-loss-of-precision.html\eslint(no-loss-of-precision)]8;;\: This number literal will lose precision at runtime. ,-[fixtures/typescript_eslint/test.ts:4:1] 3 | diff --git a/apps/oxlint/src/snapshots/fixtures__overrides_env_globals_-c .oxlintrc.json .@oxlint.snap b/apps/oxlint/src/snapshots/fixtures__overrides_env_globals_-c .oxlintrc.json .@oxlint.snap index 5dfc4a1d1665d..8d77bb22e8bfe 100644 --- a/apps/oxlint/src/snapshots/fixtures__overrides_env_globals_-c .oxlintrc.json .@oxlint.snap +++ b/apps/oxlint/src/snapshots/fixtures__overrides_env_globals_-c .oxlintrc.json .@oxlint.snap @@ -18,15 +18,6 @@ working directory: fixtures/overrides_env_globals ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-global-assign.html\eslint(no-global-assign)]8;;\: Read-only global 'globalThis' should not be modified. ,-[test.js:2:1] 1 | // for env detection - 2 | globalThis = 'abc'; - : ^^^^^|^^^^ - : `-- Read-only global 'globalThis' should not be modified. - 3 | $ = 'abc'; - `---- - - ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-global-assign.html\eslint(no-global-assign)]8;;\: Read-only global 'globalThis' should not be modified. - ,-[test.ts:2:1] - 1 | // for env detection 2 | globalThis = 'abc'; : ^^^^^|^^^^ : `-- Read-only global 'globalThis' should not be modified. @@ -50,6 +41,15 @@ working directory: fixtures/overrides_env_globals : `-- Read-only global 'Foo' should not be modified. `---- + ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-global-assign.html\eslint(no-global-assign)]8;;\: Read-only global 'globalThis' should not be modified. + ,-[test.ts:2:1] + 1 | // for env detection + 2 | globalThis = 'abc'; + : ^^^^^|^^^^ + : `-- Read-only global 'globalThis' should not be modified. + 3 | $ = 'abc'; + `---- + Found 5 warnings and 0 errors. Finished in ms on 3 files with 87 rules using 1 threads. ---------- diff --git a/apps/oxlint/src/snapshots/fixtures__overrides_with_plugin_-c .oxlintrc.json@oxlint.snap b/apps/oxlint/src/snapshots/fixtures__overrides_with_plugin_-c .oxlintrc.json@oxlint.snap index cce2536c23f46..873b1b4bec52a 100644 --- a/apps/oxlint/src/snapshots/fixtures__overrides_with_plugin_-c .oxlintrc.json@oxlint.snap +++ b/apps/oxlint/src/snapshots/fixtures__overrides_with_plugin_-c .oxlintrc.json@oxlint.snap @@ -14,15 +14,6 @@ working directory: fixtures/overrides_with_plugin `---- help: "Write a meaningful title for your test" - ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-unused-vars.html\eslint(no-unused-vars)]8;;\: Variable 'foo' is declared but never used. Unused variables should start with a '_'. - ,-[index.ts:1:7] - 1 | const foo = 123; - : ^|^ - : `-- 'foo' is declared here - 2 | // no-unused-vars error expected as `eslint` plugin and `correctness` categories are on by default (override is not applied.) - `---- - help: Consider removing this declaration. - ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/jest/expect-expect.html\eslint-plugin-jest(expect-expect)]8;;\: Test has no assertions ,-[index.test.ts:4:3] 3 | @@ -41,6 +32,15 @@ working directory: fixtures/overrides_with_plugin `---- help: "Write a meaningful title for your test" + ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-unused-vars.html\eslint(no-unused-vars)]8;;\: Variable 'foo' is declared but never used. Unused variables should start with a '_'. + ,-[index.ts:1:7] + 1 | const foo = 123; + : ^|^ + : `-- 'foo' is declared here + 2 | // no-unused-vars error expected as `eslint` plugin and `correctness` categories are on by default (override is not applied.) + `---- + help: Consider removing this declaration. + Found 2 warnings and 2 errors. Finished in ms on 2 files with 87 rules using 1 threads. ---------- diff --git a/napi/oxlint2/test/__snapshots__/e2e.test.ts.snap b/napi/oxlint2/test/__snapshots__/e2e.test.ts.snap index 342d6ec55a1b8..575e3181c78b1 100644 --- a/napi/oxlint2/test/__snapshots__/e2e.test.ts.snap +++ b/napi/oxlint2/test/__snapshots__/e2e.test.ts.snap @@ -20,18 +20,18 @@ Finished in Xms on 1 file using X threads." exports[`oxlint2 CLI > should load a custom plugin 1`] = ` " - ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html\\eslint(no-debugger)]8;;\\: \`debugger\` statement is not allowed + x basic-custom-plugin(no-debugger): Unexpected Debugger Statement ,-[index.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- - help: Remove the debugger statement - x basic-custom-plugin(no-debugger): Unexpected Debugger Statement + ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html\\eslint(no-debugger)]8;;\\: \`debugger\` statement is not allowed ,-[index.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- + help: Remove the debugger statement Found 1 warning and 1 error. Finished in Xms on 1 file using X threads." @@ -39,18 +39,18 @@ Finished in Xms on 1 file using X threads." exports[`oxlint2 CLI > should load a custom plugin when configured in overrides 1`] = ` " - ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html\\eslint(no-debugger)]8;;\\: \`debugger\` statement is not allowed + x basic-custom-plugin(no-debugger): Unexpected Debugger Statement ,-[index.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- - help: Remove the debugger statement - x basic-custom-plugin(no-debugger): Unexpected Debugger Statement + ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html\\eslint(no-debugger)]8;;\\: \`debugger\` statement is not allowed ,-[index.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- + help: Remove the debugger statement Found 1 warning and 1 error. Finished in Xms on 1 file using X threads." @@ -58,6 +58,12 @@ Finished in Xms on 1 file using X threads." exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` " + x basic-custom-plugin(no-debugger): Unexpected Debugger Statement + ,-[files/01.js:1:1] + 1 | debugger; + : ^^^^^^^^^ + \`---- + ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html\\eslint(no-debugger)]8;;\\: \`debugger\` statement is not allowed ,-[files/01.js:1:1] 1 | debugger; @@ -66,7 +72,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/01.js:1:1] + ,-[files/02.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -79,7 +85,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/02.js:1:1] + ,-[files/03.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -92,7 +98,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/03.js:1:1] + ,-[files/04.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -105,7 +111,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/04.js:1:1] + ,-[files/05.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -118,7 +124,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/05.js:1:1] + ,-[files/06.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -131,7 +137,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/06.js:1:1] + ,-[files/07.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -144,7 +150,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/07.js:1:1] + ,-[files/08.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -157,7 +163,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/08.js:1:1] + ,-[files/09.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -170,7 +176,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/09.js:1:1] + ,-[files/10.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -183,7 +189,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/10.js:1:1] + ,-[files/11.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -196,7 +202,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/11.js:1:1] + ,-[files/12.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -209,7 +215,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/12.js:1:1] + ,-[files/13.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -222,7 +228,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/13.js:1:1] + ,-[files/14.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -235,7 +241,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/14.js:1:1] + ,-[files/15.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -248,7 +254,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/15.js:1:1] + ,-[files/16.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -261,7 +267,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/16.js:1:1] + ,-[files/17.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -274,7 +280,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/17.js:1:1] + ,-[files/18.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -287,7 +293,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/18.js:1:1] + ,-[files/19.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -300,7 +306,7 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` help: Remove the debugger statement x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/19.js:1:1] + ,-[files/20.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- @@ -312,12 +318,6 @@ exports[`oxlint2 CLI > should load a custom plugin with multiple files 1`] = ` \`---- help: Remove the debugger statement - x basic-custom-plugin(no-debugger): Unexpected Debugger Statement - ,-[files/20.js:1:1] - 1 | debugger; - : ^^^^^^^^^ - \`---- - Found 20 warnings and 20 errors. Finished in Xms on 20 files using X threads." `; @@ -381,18 +381,18 @@ exports[`oxlint2 CLI > should report an error if a rule is not found within a cu exports[`oxlint2 CLI > should report the correct severity when using a custom plugin 1`] = ` " - ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html\\eslint(no-debugger)]8;;\\: \`debugger\` statement is not allowed + ! basic-custom-plugin(no-debugger): Unexpected Debugger Statement ,-[index.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- - help: Remove the debugger statement - ! basic-custom-plugin(no-debugger): Unexpected Debugger Statement + ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html\\eslint(no-debugger)]8;;\\: \`debugger\` statement is not allowed ,-[index.js:1:1] 1 | debugger; : ^^^^^^^^^ \`---- + help: Remove the debugger statement Found 2 warnings and 0 errors. Finished in Xms on 1 file using X threads." @@ -400,13 +400,12 @@ Finished in Xms on 1 file using X threads." exports[`oxlint2 CLI > should work with multiple rules 1`] = ` " - ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html\\eslint(no-debugger)]8;;\\: \`debugger\` statement is not allowed + x basic-custom-plugin(no-debugger): Unexpected Debugger Statement ,-[index.js:1:1] 1 | debugger; : ^^^^^^^^^ 2 | \`---- - help: Remove the debugger statement x basic-custom-plugin(no-debugger-2): Unexpected Debugger Statement ,-[index.js:1:1] @@ -415,12 +414,13 @@ exports[`oxlint2 CLI > should work with multiple rules 1`] = ` 2 | \`---- - x basic-custom-plugin(no-debugger): Unexpected Debugger Statement + ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html\\eslint(no-debugger)]8;;\\: \`debugger\` statement is not allowed ,-[index.js:1:1] 1 | debugger; : ^^^^^^^^^ 2 | \`---- + help: Remove the debugger statement x basic-custom-plugin(no-ident-references-named-foo): Unexpected Identifier Reference named foo ,-[index.js:3:1]