Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(syntax): give ScopeId a niche #4468

Merged
merged 1 commit into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/oxc_linter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
1 change: 1 addition & 0 deletions crates/oxc_linter/src/rules/jest/max_expects.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
77 changes: 39 additions & 38 deletions crates/oxc_linter/src/snapshots/prefer_hooks_in_order.snap
Original file line number Diff line number Diff line change
@@ -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]
Expand Down Expand Up @@ -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(() => {});
Expand All @@ -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
Expand Down Expand Up @@ -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 │
Expand All @@ -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 │ });
Expand Down Expand Up @@ -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(() => {});
Expand All @@ -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
Expand Down Expand Up @@ -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 │
Expand All @@ -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
2 changes: 1 addition & 1 deletion crates/oxc_mangler/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_semantic/src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
54 changes: 51 additions & 3 deletions crates/oxc_syntax/src/scope.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,56 @@
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`.
#[allow(clippy::missing_safety_doc, clippy::unnecessary_safety_comment)]
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<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
serializer.serialize_u32(self.0.get())
}
}

#[cfg(feature = "serialize")]
Expand Down
1 change: 1 addition & 0 deletions crates/oxc_wasm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
4 changes: 3 additions & 1 deletion crates/oxc_wasm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down
Loading