From a148023fad6df86a8b28cee1c5e6fbdc09d4f00d Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed, 23 Oct 2024 12:57:55 +0000 Subject: [PATCH] refactor(linter): dereference IDs as soon as possible (#6821) Style nit. Dereference `&ScopeId` to `ScopeId` (and other IDs) as early as possible. `&ScopeId` is 8 bytes, whereas `ScopeId` is 4 bytes. In simple cases like these, compiler will optimize it anyway, but still I think it's a better pattern to dererence early. In more complicated cases, it will be better for performance, and in my opinion, it makes things clearer if vars called `scope_id` are always a `ScopeId`, not sometimes a `&ScopeId`. --- crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs | 4 ++-- crates/oxc_linter/src/rules/jest/no_mocks_import.rs | 4 ++-- .../rules/typescript/no_unsafe_declaration_merging.rs | 8 ++++---- crates/oxc_linter/src/utils/jest.rs | 9 ++++----- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs index 1782bd28d19ed..551dd5d4cea5c 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs @@ -377,8 +377,8 @@ impl Symbol<'_, '_> { fn is_in_declare_global(&self) -> bool { self.scopes() .ancestors(self.scope_id()) - .filter(|scope_id| { - let flags = self.scopes().get_flags(*scope_id); + .filter(|&scope_id| { + let flags = self.scopes().get_flags(scope_id); flags.contains(ScopeFlags::TsModuleBlock) }) .any(|ambient_module_scope_id| { diff --git a/crates/oxc_linter/src/rules/jest/no_mocks_import.rs b/crates/oxc_linter/src/rules/jest/no_mocks_import.rs index f0889e641700f..6fc5d1fda8d7c 100644 --- a/crates/oxc_linter/src/rules/jest/no_mocks_import.rs +++ b/crates/oxc_linter/src/rules/jest/no_mocks_import.rs @@ -48,8 +48,8 @@ impl Rule for NoMocksImport { return; }; - for reference_id in require_reference_ids { - let reference = ctx.symbols().get_reference(*reference_id); + for &reference_id in require_reference_ids { + let reference = ctx.symbols().get_reference(reference_id); let Some(parent) = ctx.nodes().parent_node(reference.node_id()) else { return; }; diff --git a/crates/oxc_linter/src/rules/typescript/no_unsafe_declaration_merging.rs b/crates/oxc_linter/src/rules/typescript/no_unsafe_declaration_merging.rs index 5a2059ac00d05..f59aade117314 100644 --- a/crates/oxc_linter/src/rules/typescript/no_unsafe_declaration_merging.rs +++ b/crates/oxc_linter/src/rules/typescript/no_unsafe_declaration_merging.rs @@ -43,9 +43,9 @@ impl Rule for NoUnsafeDeclarationMerging { match node.kind() { AstKind::Class(decl) => { if let Some(ident) = decl.id.as_ref() { - for (_, symbol_id) in ctx.semantic().scopes().get_bindings(node.scope_id()) { + for (_, &symbol_id) in ctx.semantic().scopes().get_bindings(node.scope_id()) { if let AstKind::TSInterfaceDeclaration(scope_interface) = - get_symbol_kind(*symbol_id, ctx) + get_symbol_kind(symbol_id, ctx) { check_and_diagnostic(ident, &scope_interface.id, ctx); } @@ -53,8 +53,8 @@ impl Rule for NoUnsafeDeclarationMerging { } } AstKind::TSInterfaceDeclaration(decl) => { - for (_, symbol_id) in ctx.semantic().scopes().get_bindings(node.scope_id()) { - if let AstKind::Class(scope_class) = get_symbol_kind(*symbol_id, ctx) { + for (_, &symbol_id) in ctx.semantic().scopes().get_bindings(node.scope_id()) { + if let AstKind::Class(scope_class) = get_symbol_kind(symbol_id, ctx) { if let Some(scope_class_ident) = scope_class.id.as_ref() { check_and_diagnostic(&decl.id, scope_class_ident, ctx); } diff --git a/crates/oxc_linter/src/utils/jest.rs b/crates/oxc_linter/src/utils/jest.rs index 5f4bcc3fde46a..4710c9a6476a8 100644 --- a/crates/oxc_linter/src/utils/jest.rs +++ b/crates/oxc_linter/src/utils/jest.rs @@ -215,11 +215,10 @@ fn collect_ids_referenced_to_import<'a, 'c>( if matches!(import_decl.source.value.as_str(), "@jest/globals" | "vitest") { let original = find_original_name(import_decl, name); - let mut ret = vec![]; - for reference_id in reference_ids { - ret.push((*reference_id, original)); - } - + let ret = reference_ids + .iter() + .map(|&reference_id| (reference_id, original)) + .collect::>(); return Some(ret); } }