From bb3b1aa48dbf569ad525145991adff4c45938af2 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Thu, 25 Jul 2024 20:20:38 +0100 Subject: [PATCH] refactor(syntax): give `ScopeId` a niche --- Cargo.lock | 2 + crates/oxc_linter/Cargo.toml | 1 + .../oxc_linter/src/rules/jest/max_expects.rs | 1 + .../src/snapshots/prefer_hooks_in_order.snap | 77 ++++++++++--------- crates/oxc_mangler/src/lib.rs | 2 +- crates/oxc_semantic/src/scope.rs | 2 +- crates/oxc_syntax/src/scope.rs | 53 ++++++++++++- crates/oxc_wasm/Cargo.toml | 1 + crates/oxc_wasm/src/lib.rs | 4 +- 9 files changed, 99 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 110ab0a084567..73cfa92659793 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1532,6 +1532,7 @@ dependencies = [ "oxc_cfg", "oxc_codegen", "oxc_diagnostics", + "oxc_index", "oxc_macros", "oxc_parser", "oxc_resolver", @@ -1860,6 +1861,7 @@ name = "oxc_wasm" version = "0.0.0" dependencies = [ "oxc", + "oxc_index", "oxc_linter", "oxc_prettier", "serde", diff --git a/crates/oxc_linter/Cargo.toml b/crates/oxc_linter/Cargo.toml index 263e0d9c9e2cb..c0ae2bb42a83b 100644 --- a/crates/oxc_linter/Cargo.toml +++ b/crates/oxc_linter/Cargo.toml @@ -26,6 +26,7 @@ oxc_span = { workspace = true } oxc_ast = { workspace = true } oxc_cfg = { workspace = true } oxc_diagnostics = { workspace = true } +oxc_index = { workspace = true } oxc_macros = { workspace = true } oxc_semantic = { workspace = true } oxc_syntax = { workspace = true } diff --git a/crates/oxc_linter/src/rules/jest/max_expects.rs b/crates/oxc_linter/src/rules/jest/max_expects.rs index cea8f7ecff0c4..d32cc8a53bea2 100644 --- a/crates/oxc_linter/src/rules/jest/max_expects.rs +++ b/crates/oxc_linter/src/rules/jest/max_expects.rs @@ -1,5 +1,6 @@ use oxc_ast::{ast::Expression, AstKind}; use oxc_diagnostics::OxcDiagnostic; +use oxc_index::Idx; use oxc_macros::declare_oxc_lint; use oxc_span::Span; use rustc_hash::FxHashMap; diff --git a/crates/oxc_linter/src/snapshots/prefer_hooks_in_order.snap b/crates/oxc_linter/src/snapshots/prefer_hooks_in_order.snap index f19df81068c9e..1f1a2c5d547fb 100644 --- a/crates/oxc_linter/src/snapshots/prefer_hooks_in_order.snap +++ b/crates/oxc_linter/src/snapshots/prefer_hooks_in_order.snap @@ -1,5 +1,6 @@ --- source: crates/oxc_linter/src/tester.rs +assertion_line: 216 --- ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. ╭─[prefer_hooks_in_order.tsx:6:21] @@ -138,15 +139,6 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: "beforeAll" hooks should be before any "beforeEach" hooks - ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. - ╭─[prefer_hooks_in_order.tsx:10:25] - 9 │ afterEach(() => {}); - 10 │ beforeEach(() => {}); - · ──────────────────── - 11 │ afterEach(() => {}); - ╰──── - help: "beforeEach" hooks should be before any "afterEach" hooks - ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. ╭─[prefer_hooks_in_order.tsx:5:21] 4 │ afterAll(() => {}); @@ -156,6 +148,15 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: "beforeAll" hooks should be before any "afterAll" hooks + ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. + ╭─[prefer_hooks_in_order.tsx:10:25] + 9 │ afterEach(() => {}); + 10 │ beforeEach(() => {}); + · ──────────────────── + 11 │ afterEach(() => {}); + ╰──── + help: "beforeEach" hooks should be before any "afterEach" hooks + ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. ╭─[prefer_hooks_in_order.tsx:23:29] 22 │ // This comment does nothing @@ -184,16 +185,6 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: "afterEach" hooks should be before any "afterAll" hooks - ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. - ╭─[prefer_hooks_in_order.tsx:7:21] - 6 │ - 7 │ ╭─▶ beforeAll(() => { - 8 │ │ createMyDatabase(); - 9 │ ╰─▶ }); - 10 │ - ╰──── - help: "beforeAll" hooks should be before any "beforeEach" hooks - ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. ╭─[prefer_hooks_in_order.tsx:38:25] 37 │ @@ -204,6 +195,16 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: "beforeEach" hooks should be before any "afterEach" hooks + ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. + ╭─[prefer_hooks_in_order.tsx:7:21] + 6 │ + 7 │ ╭─▶ beforeAll(() => { + 8 │ │ createMyDatabase(); + 9 │ ╰─▶ }); + 10 │ + ╰──── + help: "beforeAll" hooks should be before any "beforeEach" hooks + ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. ╭─[prefer_hooks_in_order.tsx:6:17] 5 │ }); @@ -341,15 +342,6 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: "beforeAll" hooks should be before any "beforeEach" hooks - ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. - ╭─[prefer_hooks_in_order.tsx:10:21] - 9 │ afterEach(() => {}); - 10 │ beforeEach(() => {}); - · ──────────────────── - 11 │ afterEach(() => {}); - ╰──── - help: "beforeEach" hooks should be before any "afterEach" hooks - ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. ╭─[prefer_hooks_in_order.tsx:5:17] 4 │ afterAll(() => {}); @@ -359,6 +351,15 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: "beforeAll" hooks should be before any "afterAll" hooks + ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. + ╭─[prefer_hooks_in_order.tsx:10:21] + 9 │ afterEach(() => {}); + 10 │ beforeEach(() => {}); + · ──────────────────── + 11 │ afterEach(() => {}); + ╰──── + help: "beforeEach" hooks should be before any "afterEach" hooks + ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. ╭─[prefer_hooks_in_order.tsx:23:25] 22 │ // This comment does nothing @@ -387,16 +388,6 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: "afterEach" hooks should be before any "afterAll" hooks - ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. - ╭─[prefer_hooks_in_order.tsx:7:17] - 6 │ - 7 │ ╭─▶ beforeAll(() => { - 8 │ │ createMyDatabase(); - 9 │ ╰─▶ }); - 10 │ - ╰──── - help: "beforeAll" hooks should be before any "beforeEach" hooks - ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. ╭─[prefer_hooks_in_order.tsx:38:21] 37 │ @@ -406,3 +397,13 @@ source: crates/oxc_linter/src/tester.rs 41 │ ╰──── help: "beforeEach" hooks should be before any "afterEach" hooks + + ⚠ eslint-plugin-vitest(prefer-hooks-in-order): Prefer having hooks in a consistent order. + ╭─[prefer_hooks_in_order.tsx:7:17] + 6 │ + 7 │ ╭─▶ beforeAll(() => { + 8 │ │ createMyDatabase(); + 9 │ ╰─▶ }); + 10 │ + ╰──── + help: "beforeAll" hooks should be before any "beforeEach" hooks diff --git a/crates/oxc_mangler/src/lib.rs b/crates/oxc_mangler/src/lib.rs index ec42f7742d7f6..68457e1d16f66 100644 --- a/crates/oxc_mangler/src/lib.rs +++ b/crates/oxc_mangler/src/lib.rs @@ -1,6 +1,6 @@ use itertools::Itertools; use oxc_ast::ast::Program; -use oxc_index::{index_vec, IndexVec}; +use oxc_index::{index_vec, Idx, IndexVec}; use oxc_semantic::{ReferenceId, SemanticBuilder, SymbolId, SymbolTable}; use oxc_span::CompactStr; diff --git a/crates/oxc_semantic/src/scope.rs b/crates/oxc_semantic/src/scope.rs index f6663ddcd0142..24cfcc2060e8a 100644 --- a/crates/oxc_semantic/src/scope.rs +++ b/crates/oxc_semantic/src/scope.rs @@ -32,7 +32,7 @@ pub struct ScopeTree { } impl ScopeTree { - const ROOT_SCOPE_ID: ScopeId = ScopeId::from_usize_unchecked(0); + const ROOT_SCOPE_ID: ScopeId = ScopeId::new(0); pub fn len(&self) -> usize { self.parent_ids.len() diff --git a/crates/oxc_syntax/src/scope.rs b/crates/oxc_syntax/src/scope.rs index a0f9f7a12cbd7..1db9c7d9659db 100644 --- a/crates/oxc_syntax/src/scope.rs +++ b/crates/oxc_syntax/src/scope.rs @@ -1,8 +1,55 @@ use bitflags::bitflags; -use oxc_index::define_index_type; +use nonmax::NonMaxU32; +#[cfg(feature = "serialize")] +use serde::{Serialize, Serializer}; + +use oxc_index::Idx; + +#[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)] +pub struct ScopeId(NonMaxU32); + +impl ScopeId { + /// Create `ScopeId` from `u32`. + /// + /// # Panics + /// Panics if `idx` is `u32::MAX`. + pub const fn new(idx: u32) -> Self { + // We could use `NonMaxU32::new(idx).unwrap()` but `Option::unwrap` is not a const function + // and we want this function to be + assert!(idx != u32::MAX); + // SAFETY: We have checked that `idx` is not `u32::MAX` + unsafe { Self::new_unchecked(idx) } + } + + /// Create `ScopeId` from `u32` unchecked. + /// + /// # SAFETY + /// `idx` must not be `u32::MAX`. + pub const unsafe fn new_unchecked(idx: u32) -> Self { + // SAFETY: Caller must ensure `idx` is not `u32::MAX` + Self(NonMaxU32::new_unchecked(idx)) + } +} + +impl Idx for ScopeId { + #[allow(clippy::cast_possible_truncation)] + fn from_usize(idx: usize) -> Self { + Self(NonMaxU32::new(idx as u32).unwrap()) + } + + fn index(self) -> usize { + self.0.get() as usize + } +} -define_index_type! { - pub struct ScopeId = u32; +#[cfg(feature = "serialize")] +impl Serialize for ScopeId { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_u32(self.0.get()) + } } #[cfg(feature = "serialize")] diff --git a/crates/oxc_wasm/Cargo.toml b/crates/oxc_wasm/Cargo.toml index 38838aa5c9b34..ff1998272805a 100644 --- a/crates/oxc_wasm/Cargo.toml +++ b/crates/oxc_wasm/Cargo.toml @@ -21,6 +21,7 @@ doctest = false [dependencies] oxc = { workspace = true, features = ["codegen", "minifier", "semantic", "serialize", "transformer", "wasm"] } +oxc_index = { workspace = true } oxc_linter = { workspace = true } oxc_prettier = { workspace = true } serde = { workspace = true } diff --git a/crates/oxc_wasm/src/lib.rs b/crates/oxc_wasm/src/lib.rs index 18fa311a85333..10de5c2c2f5ab 100644 --- a/crates/oxc_wasm/src/lib.rs +++ b/crates/oxc_wasm/src/lib.rs @@ -16,6 +16,7 @@ use oxc::{ span::SourceType, transformer::{TransformOptions, Transformer}, }; +use oxc_index::Idx; use oxc_linter::Linter; use oxc_prettier::{Prettier, PrettierOptions}; use serde::Serialize; @@ -304,7 +305,8 @@ impl Oxc { let flag = semantic.scopes().get_flags(*scope_id); let next_scope_ids = semantic.scopes().get_child_ids(*scope_id); - scope_text.push_str(&format!("{space}Scope{:?} ({flag:?}) {{\n", *scope_id + 1)); + scope_text + .push_str(&format!("{space}Scope{:?} ({flag:?}) {{\n", scope_id.index() + 1)); let bindings = semantic.scopes().get_bindings(*scope_id); let binding_space = " ".repeat((depth + 1) * 2); if !bindings.is_empty() {