From f81e8a126e83cb74cf03cd430121a310ecc218a0 Mon Sep 17 00:00:00 2001 From: DonIsaac <22823424+DonIsaac@users.noreply.github.com> Date: Sat, 31 Aug 2024 16:59:52 +0000 Subject: [PATCH] feat(linter): add `oxc/no-async-endpoint-handlers` (#5364) Adds `no-async-endpoint-handlers` rules, which bans async functions used as endpoint handlers in Express applications. These do not get caught by Express' error handler, causing the server to crash with an unhandled process rejection error. ```js app.use(async (req, res) => { const foo = await api.getFoo(req.query) // server panics if this function rejects return res.json(foo) }) ``` I could not find this rule implemented in any ESLint plugin, but this is a problem I see quite often and I'm tired of dealing with it. I've added it to `oxc` for now, but we should consider adding an `express` or `api` plugin in the future. --- crates/oxc_ast/src/ast_impl/js.rs | 12 +- crates/oxc_linter/src/rules.rs | 2 + .../rules/oxc/no_async_endpoint_handlers.rs | 376 ++++++++++++++++++ .../snapshots/no_async_endpoint_handlers.snap | 95 +++++ crates/oxc_linter/src/utils/express.rs | 126 ++++++ crates/oxc_linter/src/utils/mod.rs | 5 +- crates/oxc_span/src/span/mod.rs | 8 + crates/oxc_transformer/src/react/refresh.rs | 8 +- 8 files changed, 625 insertions(+), 7 deletions(-) create mode 100644 crates/oxc_linter/src/rules/oxc/no_async_endpoint_handlers.rs create mode 100644 crates/oxc_linter/src/snapshots/no_async_endpoint_handlers.snap create mode 100644 crates/oxc_linter/src/utils/express.rs diff --git a/crates/oxc_ast/src/ast_impl/js.rs b/crates/oxc_ast/src/ast_impl/js.rs index e08c99fff..827662cf2 100644 --- a/crates/oxc_ast/src/ast_impl/js.rs +++ b/crates/oxc_ast/src/ast_impl/js.rs @@ -4,7 +4,9 @@ use std::{borrow::Cow, cell::Cell, fmt, hash::Hash}; use oxc_allocator::{Box, FromIn, Vec}; use oxc_span::{Atom, GetSpan, SourceType, Span}; -use oxc_syntax::{operator::UnaryOperator, reference::ReferenceId, scope::ScopeFlags}; +use oxc_syntax::{ + operator::UnaryOperator, reference::ReferenceId, scope::ScopeFlags, symbol::SymbolId, +}; #[cfg(feature = "serialize")] #[wasm_bindgen::prelude::wasm_bindgen(typescript_custom_section)] @@ -1056,6 +1058,14 @@ impl<'a> Function<'a> { self.id.as_ref().map(|id| id.name.clone()) } + /// Get the [`SymbolId`] this [`Function`] is bound to. + /// + /// Returns [`None`] for anonymous functions, or if semantic analysis was skipped. + #[inline] + pub fn symbol_id(&self) -> Option { + self.id.as_ref().and_then(|id| id.symbol_id.get()) + } + pub fn is_typescript_syntax(&self) -> bool { matches!( self.r#type, diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 57fb4f819..41798b0eb 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -390,6 +390,7 @@ mod oxc { pub mod missing_throw; pub mod no_accumulating_spread; pub mod no_async_await; + pub mod no_async_endpoint_handlers; pub mod no_barrel_file; pub mod no_const_enum; pub mod no_optional_chaining; @@ -834,6 +835,7 @@ oxc_macros::declare_all_lint_rules! { oxc::number_arg_out_of_range, oxc::only_used_in_recursion, oxc::no_async_await, + oxc::no_async_endpoint_handlers, oxc::uninvoked_array_callback, nextjs::google_font_display, nextjs::google_font_preconnect, diff --git a/crates/oxc_linter/src/rules/oxc/no_async_endpoint_handlers.rs b/crates/oxc_linter/src/rules/oxc/no_async_endpoint_handlers.rs new file mode 100644 index 000000000..d78cd0854 --- /dev/null +++ b/crates/oxc_linter/src/rules/oxc/no_async_endpoint_handlers.rs @@ -0,0 +1,376 @@ +use std::ops::Deref; + +use oxc_diagnostics::{LabeledSpan, OxcDiagnostic}; +use oxc_macros::declare_oxc_lint; +use oxc_span::{CompactStr, Span}; +use serde_json::Value; + +use crate::{context::LintContext, rule::Rule, utils, AstNode}; +use oxc_ast::{ + ast::{Argument, ArrowFunctionExpression, Expression, Function}, + AstKind, +}; + +#[derive(Debug, Default, Clone)] +pub struct NoAsyncEndpointHandlers(Box); +impl Deref for NoAsyncEndpointHandlers { + type Target = NoAsyncEndpointHandlersConfig; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +#[derive(Debug, Default, Clone)] +pub struct NoAsyncEndpointHandlersConfig { + allowed_names: Vec, +} + +pub fn no_async_handlers( + function_span: Span, + registered_span: Option, + name: Option<&str>, +) -> OxcDiagnostic { + #[allow(clippy::cast_possible_truncation)] + const ASYNC_LEN: u32 = "async".len() as u32; + + // Only cover "async" in "async function (req, res) {}" or "async (req, res) => {}" + let async_span = Span::sized(function_span.start, ASYNC_LEN); + + let labels: &[LabeledSpan] = match (registered_span, name) { + // handler is declared separately from registration + // `async function foo(req, res) {}; app.get('/foo', foo);` + (Some(span), Some(name)) => &[ + async_span.label(format!("Async handler '{name}' is declared here")), + span.primary_label("and is registered here"), + ], + // Shouldn't happen, since separate declaration/registration requires an + // identifier to be bound + (Some(span), None) => &[ + async_span.label("Async handler is declared here"), + span.primary_label("and is registered here"), + ], + // `app.get('/foo', async function foo(req, res) {});` + (None, Some(name)) => &[async_span.label(format!("Async handler '{name}' is used here"))], + + // `app.get('/foo', async (req, res) => {});` + (None, None) => &[async_span.label("Async handler is used here")], + }; + + OxcDiagnostic::warn("Express endpoint handlers should not be async.") + .with_labels(labels.iter().cloned()) + .with_help("Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead.") +} + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallows the use of `async` functions as Express endpoint handlers. + /// + /// ### Why is this bad? + /// + /// Before v5, Express will not automatically handle Promise rejections from + /// handler functions with your application's error handler. You must + /// instead explicitly pass the rejected promise to `next()`. + /// ```js + /// const app = express() + /// app.get('/', (req, res, next) => { + /// new Promise((resolve, reject) => { + /// return User.findById(req.params.id) + /// }) + /// .then(user => res.json(user)) + /// .catch(next) + /// }) + /// ``` + /// + /// If this is not done, your server will crash with an unhandled promise + /// rejection. + /// ```js + /// const app = express() + /// app.get('/', async (req, res) => { + /// // Server will crash if User.findById rejects + /// const user = await User.findById(req.params.id) + /// res.json(user) + /// }) + /// ``` + /// + /// See [Express' Error Handling + /// Guide](https://expressjs.com/en/guide/error-handling.html) for more + /// information. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// const app = express(); + /// app.get('/', async (req, res) => { + /// const user = await User.findById(req.params.id); + /// res.json(user); + /// }); + /// + /// const router = express.Router(); + /// router.use(async (req, res, next) => { + /// const user = await User.findById(req.params.id); + /// req.user = user; + /// next(); + /// }); + /// + /// const createUser = async (req, res) => { + /// const user = await User.create(req.body); + /// res.json(user); + /// } + /// app.post('/user', createUser); + /// + /// // Async handlers that are imported will not be detected because each + /// // file is checked in isolation. This does not trigger the rule, but still + /// // violates it and _will_ result in server crashes. + /// const asyncHandler = require('./asyncHandler'); + /// app.get('/async', asyncHandler); + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// const app = express(); + /// // not async + /// app.use((req, res, next) => { + /// req.receivedAt = Date.now(); + /// }) + /// + /// app.get('/', (req, res, next) => { + /// fs.readFile('/file-does-not-exist', (err, data) => { + /// if (err) { + /// next(err) // Pass errors to Express. + /// } else { + /// res.send(data) + /// } + /// }) + /// }) + /// + /// const asyncHandler = async (req, res) => { + /// const user = await User.findById(req.params.id); + /// res.json(user); + /// } + /// app.get('/user', (req, res, next) => asyncHandler(req, res).catch(next)) + /// ``` + /// + /// ## Configuration + /// + /// This rule takes the following configuration: + /// ```ts + /// type NoAsyncEndpointHandlersConfig = { + /// /** + /// * An array of names that are allowed to be async. + /// */ + /// allowedNames?: string[]; + /// } + /// ``` + NoAsyncEndpointHandlers, + suspicious +); + +impl Rule for NoAsyncEndpointHandlers { + fn from_configuration(value: Value) -> Self { + let mut allowed_names: Vec = value + .get(0) + .and_then(Value::as_object) + .and_then(|config| config.get("allowedNames")) + .and_then(Value::as_array) + .map(|names| names.iter().filter_map(Value::as_str).map(CompactStr::from).collect()) + .unwrap_or_default(); + allowed_names.sort_unstable(); + allowed_names.dedup(); + + Self(Box::new(NoAsyncEndpointHandlersConfig { allowed_names })) + } + + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let kind = node.kind(); + let Some((_endpoint, args)) = utils::as_endpoint_registration(&kind) else { + return; + }; + for arg in + args.iter().filter_map(Argument::as_expression).map(Expression::get_inner_expression) + { + self.check_endpoint_arg(ctx, arg); + } + } +} + +impl NoAsyncEndpointHandlers { + fn check_endpoint_arg<'a>(&self, ctx: &LintContext<'a>, arg: &Expression<'a>) { + self.check_endpoint_expr(ctx, None, None, arg); + } + + fn check_endpoint_expr<'a>( + &self, + ctx: &LintContext<'a>, + id_name: Option<&str>, + registered_at: Option, + arg: &Expression<'a>, + ) { + match arg { + Expression::Identifier(handler) => { + // Unresolved reference? Nothing we can do. + let Some(symbol_id) = handler + .reference_id() + .and_then(|id| ctx.symbols().get_reference(id).symbol_id()) + else { + return; + }; + + // Cannot check imported handlers without cross-file analysis. + let flags = ctx.symbols().get_flags(symbol_id); + if flags.is_import() { + return; + } + + let decl_id = ctx.symbols().get_declaration(symbol_id); + let decl_node = ctx.nodes().get_node(decl_id); + let registered_at = registered_at.or(Some(handler.span)); + match decl_node.kind() { + AstKind::Function(f) => self.check_function(ctx, registered_at, id_name, f), + AstKind::VariableDeclarator(decl) => { + if let Some(init) = &decl.init { + self.check_endpoint_expr(ctx, id_name, registered_at, init); + } + } + _ => {} + } + } + func if utils::is_endpoint_handler(func) => { + match func { + // `app.get('/', (async?) function (req, res) {}` + Expression::FunctionExpression(f) => { + self.check_function(ctx, registered_at, id_name, f); + } + Expression::ArrowFunctionExpression(f) => { + self.check_arrow(ctx, registered_at, id_name, f); + } + _ => unreachable!(), + } + } + _ => {} + } + } + + fn check_function<'a>( + &self, + ctx: &LintContext<'a>, + registered_at: Option, + id_name: Option<&str>, + f: &Function<'a>, + ) { + if !f.r#async { + return; + } + + let name = f.name().map(|n| n.as_str()).or(id_name); + if name.is_some_and(|name| self.is_allowed_name(name)) { + return; + } + + ctx.diagnostic(no_async_handlers(f.span, registered_at, name)); + } + + fn check_arrow<'a>( + &self, + ctx: &LintContext<'a>, + registered_at: Option, + id_name: Option<&str>, + f: &ArrowFunctionExpression<'a>, + ) { + if !f.r#async { + return; + } + if id_name.is_some_and(|name| self.is_allowed_name(name)) { + return; + } + + ctx.diagnostic(no_async_handlers(f.span, registered_at, id_name)); + } + + fn is_allowed_name(&self, name: &str) -> bool { + self.allowed_names.binary_search_by(|allowed| allowed.as_str().cmp(name)).is_ok() + } +} + +#[test] +fn test() { + use crate::tester::Tester; + use serde_json::json; + + let pass = vec![ + ("app.get('/', fooController)", None), + ("app.get('/', (req, res) => {})", None), + ("app.get('/', (req, res) => {})", None), + ("app.get('/', function (req, res) {})", None), + ("app.get('/', middleware, function (req, res) {})", None), + ("app.get('/', (req, res, next) => {})", None), + ("app.get('/', (err, req, res, next) => {})", None), + ("app.get('/', (err, req, res) => {})", None), + ("app.get('/', (err, req, res) => {})", None), + ("app.get('/', (req, res) => Promise.resolve())", None), + ("app.get('/', (req, res) => new Promise((resolve, reject) => resolve()))", None), + ("app.use(middleware)", None), + ("app.get(middleware)", None), + ( + "function ctl(req, res) {} + app.get(ctl)", + None, + ), + ("weirdName.get('/', async () => {})", None), + ("weirdName.get('/', async (notARequestObject) => {})", None), + // allowed names + ( + "async function ctl(req, res) {} + app.get(ctl)", + Some(json!([ { "allowedNames": ["ctl"] } ])), + ), + ( + " + async function middleware(req, res, next) {} + app.use(middleware) + ", + Some(json!([ { "allowedNames": ["middleware"] } ])), + ), + ]; + + let fail = vec![ + ("app.get('/', async function (req, res) {})", None), + ("app.get('/', async (req, res) => {})", None), + ("app.get('/', async (req, res, next) => {})", None), + ("weirdName.get('/', async (req, res) => {})", None), + ("weirdName.get('/', async (req, res) => {})", None), + ( + " + async function foo(req, res) {} + app.post('/', foo) + ", + None, + ), + ( + " + const foo = async (req, res) => {} + app.post('/', foo) + ", + None, + ), + ( + " + async function middleware(req, res, next) {} + app.use(middleware) + ", + None, + ), + ( + " + async function foo(req, res) {} + const bar = foo; + app.post('/', bar) + ", + None, + ), + ]; + + Tester::new(NoAsyncEndpointHandlers::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_async_endpoint_handlers.snap b/crates/oxc_linter/src/snapshots/no_async_endpoint_handlers.snap new file mode 100644 index 000000000..1234b7d6b --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_async_endpoint_handlers.snap @@ -0,0 +1,95 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ╭─[no_async_endpoint_handlers.tsx:1:14] + 1 │ app.get('/', async function (req, res) {}) + · ──┬── + · ╰── Async handler is used here + ╰──── + help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ╭─[no_async_endpoint_handlers.tsx:1:14] + 1 │ app.get('/', async (req, res) => {}) + · ──┬── + · ╰── Async handler is used here + ╰──── + help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ╭─[no_async_endpoint_handlers.tsx:1:14] + 1 │ app.get('/', async (req, res, next) => {}) + · ──┬── + · ╰── Async handler is used here + ╰──── + help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ╭─[no_async_endpoint_handlers.tsx:1:20] + 1 │ weirdName.get('/', async (req, res) => {}) + · ──┬── + · ╰── Async handler is used here + ╰──── + help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ╭─[no_async_endpoint_handlers.tsx:1:20] + 1 │ weirdName.get('/', async (req, res) => {}) + · ──┬── + · ╰── Async handler is used here + ╰──── + help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ╭─[no_async_endpoint_handlers.tsx:3:27] + 1 │ + 2 │ async function foo(req, res) {} + · ──┬── + · ╰── Async handler 'foo' is declared here + 3 │ app.post('/', foo) + · ─┬─ + · ╰── and is registered here + 4 │ + ╰──── + help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ╭─[no_async_endpoint_handlers.tsx:3:27] + 1 │ + 2 │ const foo = async (req, res) => {} + · ──┬── + · ╰── Async handler is declared here + 3 │ app.post('/', foo) + · ─┬─ + · ╰── and is registered here + 4 │ + ╰──── + help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ╭─[no_async_endpoint_handlers.tsx:3:21] + 1 │ + 2 │ async function middleware(req, res, next) {} + · ──┬── + · ╰── Async handler 'middleware' is declared here + 3 │ app.use(middleware) + · ─────┬──── + · ╰── and is registered here + 4 │ + ╰──── + help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ╭─[no_async_endpoint_handlers.tsx:4:27] + 1 │ + 2 │ async function foo(req, res) {} + · ──┬── + · ╰── Async handler 'foo' is declared here + 3 │ const bar = foo; + 4 │ app.post('/', bar) + · ─┬─ + · ╰── and is registered here + 5 │ + ╰──── + help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. diff --git a/crates/oxc_linter/src/utils/express.rs b/crates/oxc_linter/src/utils/express.rs new file mode 100644 index 000000000..41d19baf2 --- /dev/null +++ b/crates/oxc_linter/src/utils/express.rs @@ -0,0 +1,126 @@ +use oxc_ast::{ + ast::{Argument, Expression, FormalParameter}, + AstKind, +}; +use oxc_span::Atom; +use phf::{phf_set, set::Set}; + +/// Check if the given node is registering an endpoint handler or middleware to +/// a route or Express application object. If it is, it +/// returns: +/// - the endpoint path being handled, if found and statically analyzable +/// - the arguments to the handler function, excluding the path (if found) +/// +/// ## Example +/// ```js +/// +/// app.get('/path', (req, res) => { }); // -> Some(( Some("/path"), [Argument::Expression(Expression::Function(...))] )) +/// app.use(someMiddleware); // -> Some(( None, [Argument::Expression(Expression::IdentifierReference)] )) +/// +/// ``` +pub fn as_endpoint_registration<'a, 'n>( + node: &'n AstKind<'a>, +) -> Option<(Option>, &'n [Argument<'a>])> { + let AstKind::CallExpression(call) = node else { + return None; + }; + let callee = call.callee.as_member_expression()?; + let method_name = callee.static_property_name()?; + if !ROUTER_HANDLER_METHOD_NAMES.contains(method_name) { + return None; + } + if call.arguments.is_empty() { + return None; + } + let first = call.arguments[0].as_expression()?; + match first { + Expression::StringLiteral(path) => { + Some((Some(path.value.clone()), &call.arguments.as_slice()[1..])) + } + Expression::TemplateLiteral(template) if template.is_no_substitution_template() => { + Some((template.quasi().clone(), &call.arguments.as_slice()[1..])) + } + _ => Some((None, call.arguments.as_slice())), + } +} + +/// Check if the given expression is an endpoint handler function. +/// +/// This will yield a lot of false positives if not called on the results of +/// [`as_endpoint_registration`]. +#[allow(clippy::similar_names)] +pub fn is_endpoint_handler(maybe_handler: &Expression<'_>) -> bool { + let params = match maybe_handler { + Expression::FunctionExpression(f) => &f.params, + Expression::ArrowFunctionExpression(arrow) => &arrow.params, + _ => return false, + }; + + // NOTE(@DonIsaac): should we check for destructuring patterns? I don't + // really ever see them used in handlers, and their existence could indicate + // this function is not a handler. + if params.rest.is_some() { + return false; + } + match params.items.as_slice() { + [req] => is_req_param(req), + [req, res] => is_req_param(req) && is_res_param(res), + [req, res, next] => { + is_req_param(req) && is_res_param(res) && is_next_param(next) || + // (err, req, res) + is_error_param(req) && is_req_param(res) && is_res_param(next) + } + [err, req, res, next] => { + is_error_param(err) && is_req_param(req) && is_res_param(res) && is_next_param(next) + } + _ => false, + } +} + +const ROUTER_HANDLER_METHOD_NAMES: Set<&'static str> = phf_set! { + "get", + "post", + "put", + "delete", + "patch", + "options", + "head", + "use", + "all", +}; + +const COMMON_REQUEST_NAMES: Set<&'static str> = phf_set! { + "r", + "req", + "request", +}; +fn is_req_param(param: &FormalParameter) -> bool { + param.pattern.get_identifier().map_or(false, |id| COMMON_REQUEST_NAMES.contains(id.as_str())) +} + +const COMMON_RESPONSE_NAMES: Set<&'static str> = phf_set! { + "s", + "res", + "response", +}; +fn is_res_param(param: &FormalParameter) -> bool { + param.pattern.get_identifier().map_or(false, |id| COMMON_RESPONSE_NAMES.contains(id.as_str())) +} + +const COMMON_NEXT_NAMES: Set<&'static str> = phf_set! { + "n", + "next", +}; +fn is_next_param(param: &FormalParameter) -> bool { + param.pattern.get_identifier().map_or(false, |id| COMMON_NEXT_NAMES.contains(id.as_str())) +} + +const COMMON_ERROR_NAMES: Set<&'static str> = phf_set! { + "e", + "err", + "error", + "exception", +}; +fn is_error_param(param: &FormalParameter) -> bool { + param.pattern.get_identifier().map_or(false, |id| COMMON_ERROR_NAMES.contains(id.as_str())) +} diff --git a/crates/oxc_linter/src/utils/mod.rs b/crates/oxc_linter/src/utils/mod.rs index c9195e0d4..47e456730 100644 --- a/crates/oxc_linter/src/utils/mod.rs +++ b/crates/oxc_linter/src/utils/mod.rs @@ -1,4 +1,5 @@ mod config; +mod express; mod jest; mod jsdoc; mod nextjs; @@ -12,8 +13,8 @@ mod vitest; use std::{io, path::Path}; pub use self::{ - config::*, jest::*, jsdoc::*, nextjs::*, promise::*, react::*, react_perf::*, tree_shaking::*, - unicorn::*, vitest::*, + config::*, express::*, jest::*, jsdoc::*, nextjs::*, promise::*, react::*, react_perf::*, + tree_shaking::*, unicorn::*, vitest::*, }; /// Check if the Jest rule is adapted to Vitest. diff --git a/crates/oxc_span/src/span/mod.rs b/crates/oxc_span/src/span/mod.rs index 9a7dcfa10..669d9d187 100644 --- a/crates/oxc_span/src/span/mod.rs +++ b/crates/oxc_span/src/span/mod.rs @@ -316,10 +316,18 @@ impl Span { } /// Create a [`LabeledSpan`] covering this [`Span`] with the given label. + /// + /// Use [`Span::primary_label`] if this is the primary span for the diagnostic. #[must_use] pub fn label>(self, label: S) -> LabeledSpan { LabeledSpan::new_with_span(Some(label.into()), self) } + + /// Creates a primary [`LabeledSpan`] covering this [`Span`] with the given label. + #[must_use] + pub fn primary_label>(self, label: S) -> LabeledSpan { + LabeledSpan::new_primary_with_span(Some(label.into()), self) + } } impl Index for str { diff --git a/crates/oxc_transformer/src/react/refresh.rs b/crates/oxc_transformer/src/react/refresh.rs index fbde30276..82f372aad 100644 --- a/crates/oxc_transformer/src/react/refresh.rs +++ b/crates/oxc_transformer/src/react/refresh.rs @@ -927,7 +927,7 @@ fn get_symbol_id_from_function_and_declarator(stmt: &Statement<'_>) -> Vec { - symbol_ids.push(func.id.as_ref().unwrap().symbol_id.get().unwrap()); + symbol_ids.push(func.symbol_id().unwrap()); } Statement::VariableDeclaration(ref decl) => { symbol_ids.extend(decl.declarations.iter().filter_map(|decl| { @@ -936,7 +936,7 @@ fn get_symbol_id_from_function_and_declarator(stmt: &Statement<'_>) -> Vec { if let Some(Declaration::FunctionDeclaration(func)) = &export_decl.declaration { - symbol_ids.push(func.id.as_ref().unwrap().symbol_id.get().unwrap()); + symbol_ids.push(func.symbol_id().unwrap()); } else if let Some(Declaration::VariableDeclaration(decl)) = &export_decl.declaration { symbol_ids.extend(decl.declarations.iter().filter_map(|decl| { decl.id.get_binding_identifier().and_then(|id| id.symbol_id.get()) @@ -947,8 +947,8 @@ fn get_symbol_id_from_function_and_declarator(stmt: &Statement<'_>) -> Vec