From 2304cd8cfd6555c57ddcf3f41a2c427387a38b4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Donny/=EA=B0=95=EB=8F=99=EC=9C=A4?= Date: Thu, 27 Feb 2025 20:02:56 +0900 Subject: [PATCH] fix(es/lints): Capture errors and emit from the original thread (#10119) **Description:** This is to avoid potential regressions due to implementation details of `swc_common::errors::Emitter`. --- .changeset/plenty-worms-yawn.md | 6 + .changset/foo.md | 0 .../default/output.swc-stderr | 122 +++++++++--------- .../no-loop-func/default/output.swc-stderr | 68 +++++----- .../default/output.swc-stderr | 4 +- .../multipleDefaultExports03.1.normal.js | 26 ++-- .../multipleDefaultExports03.2.minified.js | 26 ++-- .../multipleDefaultExports04.1.normal.js | 26 ++-- .../multipleDefaultExports04.2.minified.js | 26 ++-- crates/swc_ecma_lints/src/rule.rs | 39 +++++- 10 files changed, 191 insertions(+), 152 deletions(-) create mode 100644 .changeset/plenty-worms-yawn.md delete mode 100644 .changset/foo.md diff --git a/.changeset/plenty-worms-yawn.md b/.changeset/plenty-worms-yawn.md new file mode 100644 index 000000000000..987e9eddacb9 --- /dev/null +++ b/.changeset/plenty-worms-yawn.md @@ -0,0 +1,6 @@ +--- +swc_core: patch +swc_ecma_lints: patch +--- + +fix(es/lints): Capture errors and emit from the original thread diff --git a/.changset/foo.md b/.changset/foo.md deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/crates/swc/tests/errors/lints/constructor-super/default/output.swc-stderr b/crates/swc/tests/errors/lints/constructor-super/default/output.swc-stderr index 6678b1dc9185..81bf5e002033 100644 --- a/crates/swc/tests/errors/lints/constructor-super/default/output.swc-stderr +++ b/crates/swc/tests/errors/lints/constructor-super/default/output.swc-stderr @@ -1,64 +1,3 @@ - x the name `A30` is defined multiple times - ,-[29:1] - 26 | class A27 { constructor() { for (let i = 0; i < a.length; i++) { super(); } super(); } } - 27 | class A28 extends B { constructor() { return; super(); } } - 28 | class A29 extends B { constructor() { try { super(); } catch (e) { } } } - 29 | class A30 extends B { constructor() { try { } catch (e) { super(); } } } - : ^|^ - : `-- previous definition of `A30` here - 30 | class A31 extends B { constructor() { try { } catch (e) { super(); } super(); } } - 31 | class A30 extends B { constructor() { try { super(); } catch (e) { } finally { super() } } } - : ^|^ - : `-- `A30` redefined here - 32 | - 33 | - 34 | // valid - `---- - x the name `A9` is defined multiple times - ,-[9:1] - 6 | class A6 extends (B && 5) { constructor() { super(); } } - 7 | class A7 extends (B &&= 5) { constructor() { super(); } } - 8 | class A8 extends (B += C) { constructor() { super(); } } - 9 | class A9 extends (B -= C) { constructor() { super(); } } - : ^| - : `-- previous definition of `A9` here - 10 | class A10 extends (B **= C) { constructor() { super(); } } - 11 | class A11 extends (B |= C) { constructor() { super(); } } - 12 | class A12 extends (B &= C) { constructor() { super(); } } - 13 | class A13 extends B { constructor() { } } - 14 | class A14 extends B { constructor() { for (var a of b) super.foo(); } } - 15 | class A15 extends B { constructor() { class C extends D { constructor() { super(); } } } } - 16 | class A16 extends B { constructor() { var c = class extends D { constructor() { super(); } } } } - 17 | class A17 extends B { constructor() { var c = () => super(); } } - 18 | class A18 extends B { constructor() { class C extends D { constructor() { super(); } } } } - 19 | class A19 extends B { constructor() { var C = class extends D { constructor() { super(); } } } } - 20 | class A20 extends B { constructor() { super(); class C extends D { constructor() { } } } } - 21 | class A21 extends B { constructor() { super(); var C = class extends D { constructor() { } } } } - 22 | class A23 extends B { constructor() { if (a) super(); } } - 23 | class A24 extends B { constructor() { x ? super() : null; } } - 24 | class A25 extends B { constructor() { switch (x) { case 'a': super(); } } } - 25 | class A26 { constructor() { for (let i = 0; i < a.length; i++) { super(); } } } - 26 | class A27 { constructor() { for (let i = 0; i < a.length; i++) { super(); } super(); } } - 27 | class A28 extends B { constructor() { return; super(); } } - 28 | class A29 extends B { constructor() { try { super(); } catch (e) { } } } - 29 | class A30 extends B { constructor() { try { } catch (e) { super(); } } } - 30 | class A31 extends B { constructor() { try { } catch (e) { super(); } super(); } } - 31 | class A30 extends B { constructor() { try { super(); } catch (e) { } finally { super() } } } - 32 | - 33 | - 34 | // valid - 35 | class V1 extends (B, C) { constructor() { super(); } } - 36 | class V2 extends (class B { }) { constructor() { super(); } } - 37 | class V3 { constructor() { class B extends C { constructor() { super(); } } } } - 38 | class V4 extends Object { constructor() { super(); for (let i = 0; i < 0; i++); } } - 39 | class V5 { } - 40 | class V6 { constructor() { } } - 41 | class V7 extends null { } - 42 | class V8 { constructor() { } } - 43 | class A9 extends B { constructor() { try { } finally { super(); } } } - : ^| - : `-- `A9` redefined here - `---- x Unexpected 'super()' because 'super' is not a constructor ,-[1:1] 1 | class A1 extends null { constructor() { super(); } } @@ -381,6 +320,22 @@ 31 | class A30 extends B { constructor() { try { super(); } catch (e) { } finally { super() } } } 32 | `---- + x the name `A30` is defined multiple times + ,-[29:1] + 26 | class A27 { constructor() { for (let i = 0; i < a.length; i++) { super(); } super(); } } + 27 | class A28 extends B { constructor() { return; super(); } } + 28 | class A29 extends B { constructor() { try { super(); } catch (e) { } } } + 29 | class A30 extends B { constructor() { try { } catch (e) { super(); } } } + : ^|^ + : `-- previous definition of `A30` here + 30 | class A31 extends B { constructor() { try { } catch (e) { super(); } super(); } } + 31 | class A30 extends B { constructor() { try { super(); } catch (e) { } finally { super() } } } + : ^|^ + : `-- `A30` redefined here + 32 | + 33 | + 34 | // valid + `---- x Unexpected duplicate 'super()' ,-[31:1] 28 | class A29 extends B { constructor() { try { super(); } catch (e) { } } } @@ -392,3 +347,48 @@ 33 | 34 | // valid `---- + x the name `A9` is defined multiple times + ,-[9:1] + 6 | class A6 extends (B && 5) { constructor() { super(); } } + 7 | class A7 extends (B &&= 5) { constructor() { super(); } } + 8 | class A8 extends (B += C) { constructor() { super(); } } + 9 | class A9 extends (B -= C) { constructor() { super(); } } + : ^| + : `-- previous definition of `A9` here + 10 | class A10 extends (B **= C) { constructor() { super(); } } + 11 | class A11 extends (B |= C) { constructor() { super(); } } + 12 | class A12 extends (B &= C) { constructor() { super(); } } + 13 | class A13 extends B { constructor() { } } + 14 | class A14 extends B { constructor() { for (var a of b) super.foo(); } } + 15 | class A15 extends B { constructor() { class C extends D { constructor() { super(); } } } } + 16 | class A16 extends B { constructor() { var c = class extends D { constructor() { super(); } } } } + 17 | class A17 extends B { constructor() { var c = () => super(); } } + 18 | class A18 extends B { constructor() { class C extends D { constructor() { super(); } } } } + 19 | class A19 extends B { constructor() { var C = class extends D { constructor() { super(); } } } } + 20 | class A20 extends B { constructor() { super(); class C extends D { constructor() { } } } } + 21 | class A21 extends B { constructor() { super(); var C = class extends D { constructor() { } } } } + 22 | class A23 extends B { constructor() { if (a) super(); } } + 23 | class A24 extends B { constructor() { x ? super() : null; } } + 24 | class A25 extends B { constructor() { switch (x) { case 'a': super(); } } } + 25 | class A26 { constructor() { for (let i = 0; i < a.length; i++) { super(); } } } + 26 | class A27 { constructor() { for (let i = 0; i < a.length; i++) { super(); } super(); } } + 27 | class A28 extends B { constructor() { return; super(); } } + 28 | class A29 extends B { constructor() { try { super(); } catch (e) { } } } + 29 | class A30 extends B { constructor() { try { } catch (e) { super(); } } } + 30 | class A31 extends B { constructor() { try { } catch (e) { super(); } super(); } } + 31 | class A30 extends B { constructor() { try { super(); } catch (e) { } finally { super() } } } + 32 | + 33 | + 34 | // valid + 35 | class V1 extends (B, C) { constructor() { super(); } } + 36 | class V2 extends (class B { }) { constructor() { super(); } } + 37 | class V3 { constructor() { class B extends C { constructor() { super(); } } } } + 38 | class V4 extends Object { constructor() { super(); for (let i = 0; i < 0; i++); } } + 39 | class V5 { } + 40 | class V6 { constructor() { } } + 41 | class V7 extends null { } + 42 | class V8 { constructor() { } } + 43 | class A9 extends B { constructor() { try { } finally { super(); } } } + : ^| + : `-- `A9` redefined here + `---- diff --git a/crates/swc/tests/errors/lints/no-loop-func/default/output.swc-stderr b/crates/swc/tests/errors/lints/no-loop-func/default/output.swc-stderr index 786bed27e72c..3a311f38fa66 100644 --- a/crates/swc/tests/errors/lints/no-loop-func/default/output.swc-stderr +++ b/crates/swc/tests/errors/lints/no-loop-func/default/output.swc-stderr @@ -1,37 +1,3 @@ - x the name `a` is defined multiple times - ,-[36:1] - 33 | } - 34 | - 35 | // Not ok - 36 | let a; for (let i in {}) { (function() { a; }); a = 1; } - : | - : `-- previous definition of `a` here - 37 | - 38 | // interview example =) - 39 | for (var i = 0; i < 10; i++) { - 40 | setTimeout(() => { - 41 | console.log(i); - 42 | }) - 43 | } - 44 | - 45 | // it's ok - 46 | while (cond) { - 47 | let x = 10; - 48 | - 49 | function ee() { - 50 | alert(x); - 51 | } - 52 | } - 53 | - 54 | // not ok - 55 | while (true) { - 56 | var a = 0; - : | - : `-- `a` redefined here - 57 | - 58 | while (true) { - 59 | setTimeout(() => { - `---- x Function declared in a loop contains unsafe references to variable i ,-[9:1] 6 | @@ -77,6 +43,40 @@ 44 | 45 | // it's ok `---- + x the name `a` is defined multiple times + ,-[36:1] + 33 | } + 34 | + 35 | // Not ok + 36 | let a; for (let i in {}) { (function() { a; }); a = 1; } + : | + : `-- previous definition of `a` here + 37 | + 38 | // interview example =) + 39 | for (var i = 0; i < 10; i++) { + 40 | setTimeout(() => { + 41 | console.log(i); + 42 | }) + 43 | } + 44 | + 45 | // it's ok + 46 | while (cond) { + 47 | let x = 10; + 48 | + 49 | function ee() { + 50 | alert(x); + 51 | } + 52 | } + 53 | + 54 | // not ok + 55 | while (true) { + 56 | var a = 0; + : | + : `-- `a` redefined here + 57 | + 58 | while (true) { + 59 | setTimeout(() => { + `---- x Function declared in a loop contains unsafe references to variable a ,-[59:1] 56 | var a = 0; diff --git a/crates/swc/tests/errors/lints/no-prototype-builtins/default/output.swc-stderr b/crates/swc/tests/errors/lints/no-prototype-builtins/default/output.swc-stderr index f0ae4f83a032..de28ab57b229 100644 --- a/crates/swc/tests/errors/lints/no-prototype-builtins/default/output.swc-stderr +++ b/crates/swc/tests/errors/lints/no-prototype-builtins/default/output.swc-stderr @@ -140,7 +140,7 @@ 27 | (foo?.anotherProp, foo?.hasOwnProperty)('bar'); 28 | 29 | (foo.hasOwnProperty('ok'), foo?.hasOwnProperty)('bar'); - : ^^^^^^^^^^^^^^ + : ^^^^^^^^^^^^^^ 30 | 31 | foo['hasOwnProperty']('bar'); `---- @@ -150,7 +150,7 @@ 27 | (foo?.anotherProp, foo?.hasOwnProperty)('bar'); 28 | 29 | (foo.hasOwnProperty('ok'), foo?.hasOwnProperty)('bar'); - : ^^^^^^^^^^^^^^ + : ^^^^^^^^^^^^^^ 30 | 31 | foo['hasOwnProperty']('bar'); `---- diff --git a/crates/swc/tests/tsc-references/multipleDefaultExports03.1.normal.js b/crates/swc/tests/tsc-references/multipleDefaultExports03.1.normal.js index bd598a8f1c5a..1422bba8c82d 100644 --- a/crates/swc/tests/tsc-references/multipleDefaultExports03.1.normal.js +++ b/crates/swc/tests/tsc-references/multipleDefaultExports03.1.normal.js @@ -1,17 +1,4 @@ //// [multipleDefaultExports03.ts] -//! x the name `C` is defined multiple times -//! ,-[2:1] -//! 1 | -//! 2 | export default class C { -//! : | -//! : `-- previous definition of `C` here -//! 3 | } -//! 4 | -//! 5 | export default class C { -//! : | -//! : `-- `C` redefined here -//! 6 | } -//! `---- //! x the name `default` is exported multiple times //! ,-[2:1] //! 1 | @@ -25,3 +12,16 @@ //! `---- //! //! Advice: > Exported identifiers must be unique +//! x the name `C` is defined multiple times +//! ,-[2:1] +//! 1 | +//! 2 | export default class C { +//! : | +//! : `-- previous definition of `C` here +//! 3 | } +//! 4 | +//! 5 | export default class C { +//! : | +//! : `-- `C` redefined here +//! 6 | } +//! `---- diff --git a/crates/swc/tests/tsc-references/multipleDefaultExports03.2.minified.js b/crates/swc/tests/tsc-references/multipleDefaultExports03.2.minified.js index bd598a8f1c5a..1422bba8c82d 100644 --- a/crates/swc/tests/tsc-references/multipleDefaultExports03.2.minified.js +++ b/crates/swc/tests/tsc-references/multipleDefaultExports03.2.minified.js @@ -1,17 +1,4 @@ //// [multipleDefaultExports03.ts] -//! x the name `C` is defined multiple times -//! ,-[2:1] -//! 1 | -//! 2 | export default class C { -//! : | -//! : `-- previous definition of `C` here -//! 3 | } -//! 4 | -//! 5 | export default class C { -//! : | -//! : `-- `C` redefined here -//! 6 | } -//! `---- //! x the name `default` is exported multiple times //! ,-[2:1] //! 1 | @@ -25,3 +12,16 @@ //! `---- //! //! Advice: > Exported identifiers must be unique +//! x the name `C` is defined multiple times +//! ,-[2:1] +//! 1 | +//! 2 | export default class C { +//! : | +//! : `-- previous definition of `C` here +//! 3 | } +//! 4 | +//! 5 | export default class C { +//! : | +//! : `-- `C` redefined here +//! 6 | } +//! `---- diff --git a/crates/swc/tests/tsc-references/multipleDefaultExports04.1.normal.js b/crates/swc/tests/tsc-references/multipleDefaultExports04.1.normal.js index c86cfa592e2d..6c82a75e0d78 100644 --- a/crates/swc/tests/tsc-references/multipleDefaultExports04.1.normal.js +++ b/crates/swc/tests/tsc-references/multipleDefaultExports04.1.normal.js @@ -1,17 +1,4 @@ //// [multipleDefaultExports04.ts] -//! x the name `f` is defined multiple times -//! ,-[2:1] -//! 1 | -//! 2 | export default function f() { -//! : | -//! : `-- previous definition of `f` here -//! 3 | } -//! 4 | -//! 5 | export default function f() { -//! : | -//! : `-- `f` redefined here -//! 6 | } -//! `---- //! x the name `default` is exported multiple times //! ,-[2:1] //! 1 | @@ -25,3 +12,16 @@ //! `---- //! //! Advice: > Exported identifiers must be unique +//! x the name `f` is defined multiple times +//! ,-[2:1] +//! 1 | +//! 2 | export default function f() { +//! : | +//! : `-- previous definition of `f` here +//! 3 | } +//! 4 | +//! 5 | export default function f() { +//! : | +//! : `-- `f` redefined here +//! 6 | } +//! `---- diff --git a/crates/swc/tests/tsc-references/multipleDefaultExports04.2.minified.js b/crates/swc/tests/tsc-references/multipleDefaultExports04.2.minified.js index c86cfa592e2d..6c82a75e0d78 100644 --- a/crates/swc/tests/tsc-references/multipleDefaultExports04.2.minified.js +++ b/crates/swc/tests/tsc-references/multipleDefaultExports04.2.minified.js @@ -1,17 +1,4 @@ //// [multipleDefaultExports04.ts] -//! x the name `f` is defined multiple times -//! ,-[2:1] -//! 1 | -//! 2 | export default function f() { -//! : | -//! : `-- previous definition of `f` here -//! 3 | } -//! 4 | -//! 5 | export default function f() { -//! : | -//! : `-- `f` redefined here -//! 6 | } -//! `---- //! x the name `default` is exported multiple times //! ,-[2:1] //! 1 | @@ -25,3 +12,16 @@ //! `---- //! //! Advice: > Exported identifiers must be unique +//! x the name `f` is defined multiple times +//! ,-[2:1] +//! 1 | +//! 2 | export default function f() { +//! : | +//! : `-- previous definition of `f` here +//! 3 | } +//! 4 | +//! 5 | export default function f() { +//! : | +//! : `-- `f` redefined here +//! 6 | } +//! `---- diff --git a/crates/swc_ecma_lints/src/rule.rs b/crates/swc_ecma_lints/src/rule.rs index d77f3ba8b784..8f686cffb35e 100644 --- a/crates/swc_ecma_lints/src/rule.rs +++ b/crates/swc_ecma_lints/src/rule.rs @@ -1,7 +1,11 @@ -use std::fmt::Debug; +use std::{fmt::Debug, mem::take, sync::Arc}; use auto_impl::auto_impl; -use swc_common::{errors::HANDLER, GLOBALS}; +use parking_lot::Mutex; +use swc_common::{ + errors::{Diagnostic, DiagnosticBuilder, Emitter, Handler, HANDLER}, + GLOBALS, +}; use swc_ecma_ast::{Module, Script}; use swc_ecma_visit::{Visit, VisitWith}; use swc_parallel::join; @@ -75,7 +79,36 @@ fn lint_rules, R: Rule>(rules: &mut Vec, program: &N) { program.lint(rule); } } else { - join_lint_rules(rules, program); + let capturing = Capturing::default(); + + { + HANDLER.set( + &Handler::with_emitter(true, false, Box::new(capturing.clone())), + || { + join_lint_rules(rules, program); + }, + ); + + let mut errors = take(&mut *capturing.errors.lock()); + errors.sort_by_key(|error| error.span.primary_span()); + + HANDLER.with(|handler| { + for error in errors { + DiagnosticBuilder::new_diagnostic(handler, error).emit(); + } + }); + } + } +} + +#[derive(Default, Clone)] +struct Capturing { + errors: Arc>>, +} + +impl Emitter for Capturing { + fn emit(&mut self, db: &DiagnosticBuilder<'_>) { + self.errors.lock().push((**db).clone()); } }