Skip to content

Commit

Permalink
refactor(ast)!: IdentifierReference::reference_id return `Reference…
Browse files Browse the repository at this point in the history
…Id` (#7126)

Alter `IdentifierReference::reference_id` to return `ReferenceId`, instead of `Option<ReferenceId>`.

This method is only useful on a post-semantic AST, where it will never return `None`. Returning `ReferenceId` discourages the anti-pattern of treating the result as if it could be either `Some` or `None`, and shortens code.
  • Loading branch information
overlookmotel committed Nov 5, 2024
1 parent ff8bd50 commit 843bce4
Show file tree
Hide file tree
Showing 14 changed files with 29 additions and 36 deletions.
11 changes: 8 additions & 3 deletions crates/oxc_ast/src/ast_impl/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,15 @@ impl<'a> fmt::Display for IdentifierName<'a> {
}

impl<'a> IdentifierReference<'a> {
/// Get `ReferenceId` of `IdentifierReference`.
///
/// Only use this method on a post-semantic AST where `ReferenceId`s are always defined.
///
/// # Panics
/// Panics if `reference_id` is `None`.
#[inline]
#[allow(missing_docs)]
pub fn reference_id(&self) -> Option<ReferenceId> {
self.reference_id.get()
pub fn reference_id(&self) -> ReferenceId {
self.reference_id.get().unwrap()
}
}

Expand Down
5 changes: 1 addition & 4 deletions crates/oxc_linter/src/rules/eslint/no_throw_literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,7 @@ impl NoThrowLiteral {
|| Self::could_be_error(ctx, &expr.alternate)
}
Expression::Identifier(ident) => {
let Some(ref_id) = ident.reference_id() else {
return true;
};
let reference = ctx.symbols().get_reference(ref_id);
let reference = ctx.symbols().get_reference(ident.reference_id());
let Some(symbol_id) = reference.symbol_id() else {
return true;
};
Expand Down
5 changes: 2 additions & 3 deletions crates/oxc_linter/src/rules/oxc/no_async_endpoint_handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,8 @@ impl NoAsyncEndpointHandlers {
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())
let Some(symbol_id) =
ctx.symbols().get_reference(handler.reference_id()).symbol_id()
else {
return;
};
Expand Down
16 changes: 7 additions & 9 deletions crates/oxc_linter/src/rules/oxc/no_map_spread.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use oxc_semantic::{Reference, ReferenceId, ScopeId, SymbolId};
use oxc_semantic::{ReferenceId, ScopeId, SymbolId};
use serde::{Deserialize, Serialize};
use std::ops::Deref;

Expand Down Expand Up @@ -354,10 +354,10 @@ impl Rule for NoMapSpread {

match leftmost_identifier_reference(&call_expr.callee) {
Ok(ident) => {
if let Some(ref_id) = ident.reference_id() {
if self.is_ignored_map_call(ctx, ident.name.as_str(), ref_id, call_expr.span) {
return;
}
let reference_id = ident.reference_id();
if self.is_ignored_map_call(ctx, ident.name.as_str(), reference_id, call_expr.span)
{
return;
}
}
// Mapped class properties likely have their elements spread to
Expand Down Expand Up @@ -675,10 +675,8 @@ where
// check if identifier is a reference to a spread-initialized
// variable declared within the map callback.
Expression::Identifier(ident) => {
let Some(symbol_id) = ident
.reference_id()
.map(|id| self.ctx.symbols().get_reference(id))
.and_then(Reference::symbol_id)
let Some(symbol_id) =
self.ctx.symbols().get_reference(ident.reference_id()).symbol_id()
else {
return;
};
Expand Down
3 changes: 1 addition & 2 deletions crates/oxc_linter/src/rules/oxc/only_used_in_recursion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,7 @@ fn is_recursive_call(
ctx: &LintContext,
) -> bool {
if let Expression::Identifier(identifier) = &call_expr.callee {
if let Some(symbol_id) =
identifier.reference_id().and_then(|id| ctx.symbols().get_reference(id).symbol_id())
if let Some(symbol_id) = ctx.symbols().get_reference(identifier.reference_id()).symbol_id()
{
return symbol_id == function_symbol_id;
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/react/jsx_no_undef.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl Rule for JsxNoUndef {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
if let AstKind::JSXOpeningElement(elem) = &node.kind() {
if let Some(ident) = get_resolvable_ident(&elem.name) {
let reference = ctx.symbols().get_reference(ident.reference_id().unwrap());
let reference = ctx.symbols().get_reference(ident.reference_id());
if reference.symbol_id().is_some() {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl Rule for NoWrapperObjectTypes {
};

if matches!(ident_name, "BigInt" | "Boolean" | "Number" | "Object" | "String" | "Symbol") {
if reference_id.and_then(|v| ctx.symbols().get_reference(v).symbol_id()).is_some() {
if ctx.symbols().get_reference(reference_id).symbol_id().is_some() {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ struct ReferencesFinder {

impl<'a> Visit<'a> for ReferencesFinder {
fn visit_identifier_reference(&mut self, it: &oxc_ast::ast::IdentifierReference<'a>) {
self.references.push(it.reference_id().unwrap());
self.references.push(it.reference_id());
}

fn visit_jsx_element_name(&mut self, _it: &oxc_ast::ast::JSXElementName<'a>) {
Expand Down
4 changes: 1 addition & 3 deletions crates/oxc_linter/src/utils/react_perf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,7 @@ where
let Expression::Identifier(ident) = expr else {
return;
};
let Some(symbol_id) =
ident.reference_id().and_then(|id| ctx.symbols().get_reference(id).symbol_id())
else {
let Some(symbol_id) = ctx.symbols().get_reference(ident.reference_id()).symbol_id() else {
return;
};
// Symbols declared at the root scope won't (or, at least, shouldn't) be
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl ConformanceTest for IdentifierReferenceTest {
let AstKind::IdentifierReference(id) = node.kind() else {
return TestResult::Pass;
};
let Some(reference_id) = id.reference_id() else {
let Some(reference_id) = id.reference_id.get() else {
return missing_reference_id(id);
};

Expand Down
5 changes: 2 additions & 3 deletions crates/oxc_transformer/src/es2016/exponentiation_operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl<'a, 'ctx> ExponentiationOperator<'a, 'ctx> {
let mut temp_var_inits = ctx.ast.vec();

// Make sure side-effects of evaluating `left` only happen once
let reference = ctx.scoping.symbols_mut().get_reference_mut(ident.reference_id().unwrap());
let reference = ctx.scoping.symbols_mut().get_reference_mut(ident.reference_id());
let pow_left =
if let Some(symbol_id) = reference.symbol_id() {
// This variable is declared in scope so evaluating it multiple times can't trigger a getter.
Expand Down Expand Up @@ -492,8 +492,7 @@ impl<'a, 'ctx> ExponentiationOperator<'a, 'ctx> {
match obj {
Expression::Super(super_) => return ctx.ast.expression_super(super_.span),
Expression::Identifier(ident) => {
let symbol_id =
ctx.symbols().get_reference(ident.reference_id().unwrap()).symbol_id();
let symbol_id = ctx.symbols().get_reference(ident.reference_id()).symbol_id();
if let Some(symbol_id) = symbol_id {
// This variable is declared in scope so evaluating it multiple times can't trigger a getter.
// No need for a temp var.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ impl<'a, 'ctx> LogicalAssignmentOperators<'a, 'ctx> {
ident: &IdentifierReference<'a>,
ctx: &mut TraverseCtx<'a>,
) -> (Expression<'a>, AssignmentTarget<'a>) {
let reference = ctx.symbols_mut().get_reference_mut(ident.reference_id().unwrap());
let reference = ctx.symbols_mut().get_reference_mut(ident.reference_id());
*reference.flags_mut() = ReferenceFlags::Read;
let symbol_id = reference.symbol_id();
let left_expr = Expression::Identifier(ctx.alloc(ident.clone()));
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_transformer/src/es2022/class_static_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ struct ReferenceFlagsSetter<'s> {

impl<'a, 's> Visit<'a> for ReferenceFlagsSetter<'s> {
fn visit_identifier_reference(&mut self, ident: &IdentifierReference<'a>) {
let reference_id = ident.reference_id().unwrap();
let reference_id = ident.reference_id();
let reference = self.symbols.get_reference_mut(reference_id);
*reference.flags_mut() |= ReferenceFlags::Read;
}
Expand Down
4 changes: 1 addition & 3 deletions crates/oxc_traverse/src/context/scoping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,10 +347,8 @@ impl TraverseScoping {
}

/// Delete reference for an `IdentifierReference`.
#[allow(clippy::missing_panics_doc)]
pub fn delete_reference_for_identifier(&mut self, ident: &IdentifierReference) {
// `unwrap` should never panic as `IdentifierReference`s should always have a `ReferenceId`
self.delete_reference(ident.reference_id().unwrap(), &ident.name);
self.delete_reference(ident.reference_id(), &ident.name);
}

/// Determine whether evaluating the specific input `node` is a consequenceless reference.
Expand Down

0 comments on commit 843bce4

Please sign in to comment.