From 067f9b5a6f4fb97b7d2c3a8919a1565e98b68469 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Tue, 10 Sep 2024 10:12:08 +0000 Subject: [PATCH] refactor(semantic): introduce `IsGlobalReference` trait (#5672) --- crates/oxc_linter/src/ast_util.rs | 20 +------ .../src/rules/eslint/no_new_func.rs | 12 +--- crates/oxc_linter/src/rules/eslint/radix.rs | 11 ++-- .../src/rules/node/no_exports_assign.rs | 15 ++--- crates/oxc_minifier/src/node_util/mod.rs | 4 +- crates/oxc_semantic/src/lib.rs | 2 +- crates/oxc_semantic/src/symbol.rs | 55 +++++++++++++++---- 7 files changed, 61 insertions(+), 58 deletions(-) diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index d3e9cd26c2d22..2faf06477c9f9 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -1,5 +1,5 @@ use oxc_ast::{ast::BindingIdentifier, AstKind}; -use oxc_semantic::{AstNode, AstNodeId, SymbolId}; +use oxc_semantic::{AstNode, AstNodeId, IsGlobalReference, SymbolId}; use oxc_span::{GetSpan, Span}; use oxc_syntax::operator::{AssignmentOperator, BinaryOperator, LogicalOperator, UnaryOperator}; @@ -384,27 +384,11 @@ pub fn get_new_expr_ident_name<'a>(new_expr: &'a NewExpression<'a>) -> Option<&' Some(ident.name.as_str()) } -/// Check if the given [IdentifierReference] is a global reference. -/// Such as `window`, `document`, `globalThis`, etc. -pub fn is_global_reference(ident: &IdentifierReference, ctx: &LintContext) -> bool { - let symbol_table = ctx.semantic().symbols(); - let Some(reference_id) = ident.reference_id.get() else { - return false; - }; - let reference = symbol_table.get_reference(reference_id); - reference.symbol_id().is_none() -} - pub fn is_global_require_call(call_expr: &CallExpression, ctx: &LintContext) -> bool { if call_expr.arguments.len() != 1 { return false; } - - if let Expression::Identifier(id_ref) = &call_expr.callee { - id_ref.name == "require" && is_global_reference(id_ref, ctx) - } else { - false - } + call_expr.callee.is_global_reference_name("require", ctx.symbols()) } pub fn is_function_node(node: &AstNode) -> bool { diff --git a/crates/oxc_linter/src/rules/eslint/no_new_func.rs b/crates/oxc_linter/src/rules/eslint/no_new_func.rs index fe195efbd27f4..7da90da7d9c05 100644 --- a/crates/oxc_linter/src/rules/eslint/no_new_func.rs +++ b/crates/oxc_linter/src/rules/eslint/no_new_func.rs @@ -1,6 +1,7 @@ -use oxc_ast::{ast::IdentifierReference, AstKind}; +use oxc_ast::AstKind; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; +use oxc_semantic::IsGlobalReference; use oxc_span::Span; use crate::{context::LintContext, rule::Rule, AstNode}; @@ -9,13 +10,6 @@ fn no_new_func(span: Span) -> OxcDiagnostic { OxcDiagnostic::warn("The Function constructor is eval.").with_label(span) } -fn is_global_function_reference(ctx: &LintContext, id: &IdentifierReference) -> bool { - if let Some(reference_id) = id.reference_id() { - return id.name == "Function" && ctx.symbols().is_global_reference(reference_id); - } - false -} - #[derive(Debug, Default, Clone)] pub struct NoNewFunc; @@ -99,7 +93,7 @@ impl Rule for NoNewFunc { }; if let Some((id, span)) = id_and_span { - if is_global_function_reference(ctx, id) { + if id.is_global_reference_name("Function", ctx.symbols()) { ctx.diagnostic(no_new_func(span)); } } diff --git a/crates/oxc_linter/src/rules/eslint/radix.rs b/crates/oxc_linter/src/rules/eslint/radix.rs index 07fb0f6c1387b..383c62ef2a18f 100644 --- a/crates/oxc_linter/src/rules/eslint/radix.rs +++ b/crates/oxc_linter/src/rules/eslint/radix.rs @@ -4,6 +4,7 @@ use oxc_ast::{ }; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; +use oxc_semantic::IsGlobalReference; use oxc_span::{GetSpan, Span}; use crate::{context::LintContext, rule::Rule, AstNode}; @@ -72,17 +73,14 @@ impl Rule for Radix { match call_expr.callee.without_parentheses() { Expression::Identifier(ident) => { - if ident.name == "parseInt" - && ctx.symbols().is_global_reference(ident.reference_id().unwrap()) - { + if ident.is_global_reference_name("parseInt", ctx.symbols()) { Self::check_arguments(self, call_expr, ctx); } } Expression::StaticMemberExpression(member_expr) => { if let Expression::Identifier(ident) = member_expr.object.without_parentheses() { - if ident.name == "Number" + if ident.is_global_reference_name("Number", ctx.symbols()) && member_expr.property.name == "parseInt" - && ctx.symbols().is_global_reference(ident.reference_id().unwrap()) { Self::check_arguments(self, call_expr, ctx); } @@ -91,9 +89,8 @@ impl Rule for Radix { Expression::ChainExpression(chain_expr) => { if let Some(member_expr) = chain_expr.expression.as_member_expression() { if let Expression::Identifier(ident) = member_expr.object() { - if ident.name == "Number" + if ident.is_global_reference_name("Number", ctx.symbols()) && member_expr.static_property_name() == Some("parseInt") - && ctx.symbols().is_global_reference(ident.reference_id().unwrap()) { Self::check_arguments(self, call_expr, ctx); } diff --git a/crates/oxc_linter/src/rules/node/no_exports_assign.rs b/crates/oxc_linter/src/rules/node/no_exports_assign.rs index ebdec46cc9c86..616f7f3660037 100644 --- a/crates/oxc_linter/src/rules/node/no_exports_assign.rs +++ b/crates/oxc_linter/src/rules/node/no_exports_assign.rs @@ -1,9 +1,10 @@ use oxc_ast::{ - ast::{AssignmentTarget, Expression, IdentifierReference, MemberExpression}, + ast::{AssignmentTarget, Expression, MemberExpression}, AstKind, }; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; +use oxc_semantic::IsGlobalReference; use oxc_span::{GetSpan, Span}; use crate::{context::LintContext, rule::Rule, AstNode}; @@ -14,19 +15,11 @@ fn no_exports_assign(span: Span) -> OxcDiagnostic { .with_help("Unexpected assignment to 'exports' variable. Use 'module.exports' instead.") } -fn is_global_reference(ctx: &LintContext, id: &IdentifierReference, name: &str) -> bool { - if let Some(reference_id) = id.reference_id() { - return id.name == name && ctx.symbols().is_global_reference(reference_id); - } - false -} - fn is_exports(node: &AssignmentTarget, ctx: &LintContext) -> bool { let AssignmentTarget::AssignmentTargetIdentifier(id) = node else { return false; }; - - is_global_reference(ctx, id, "exports") + id.is_global_reference_name("exports", ctx.symbols()) } fn is_module_exports(expr: Option<&MemberExpression>, ctx: &LintContext) -> bool { @@ -39,7 +32,7 @@ fn is_module_exports(expr: Option<&MemberExpression>, ctx: &LintContext) -> bool }; return mem_expr.static_property_name() == Some("exports") - && is_global_reference(ctx, obj_id, "module"); + && obj_id.is_global_reference_name("module", ctx.symbols()); } #[derive(Debug, Default, Clone)] diff --git a/crates/oxc_minifier/src/node_util/mod.rs b/crates/oxc_minifier/src/node_util/mod.rs index 15f8f7c810815..c63fbcf9c16bc 100644 --- a/crates/oxc_minifier/src/node_util/mod.rs +++ b/crates/oxc_minifier/src/node_util/mod.rs @@ -8,7 +8,7 @@ use std::borrow::Cow; use num_bigint::BigInt; use num_traits::{One, Zero}; use oxc_ast::ast::*; -use oxc_semantic::{ScopeTree, SymbolTable}; +use oxc_semantic::{IsGlobalReference, ScopeTree, SymbolTable}; use oxc_syntax::operator::{AssignmentOperator, LogicalOperator, UnaryOperator}; pub use self::{ @@ -34,7 +34,7 @@ pub trait NodeUtil { } fn is_identifier_undefined(&self, ident: &IdentifierReference) -> bool { - if ident.name == "undefined" && self.symbols().is_global_identifier_reference(ident) { + if ident.name == "undefined" && ident.is_global_reference(self.symbols()) { return true; } false diff --git a/crates/oxc_semantic/src/lib.rs b/crates/oxc_semantic/src/lib.rs index a506e75495288..b599457e1c3b4 100644 --- a/crates/oxc_semantic/src/lib.rs +++ b/crates/oxc_semantic/src/lib.rs @@ -40,7 +40,7 @@ pub use oxc_syntax::{ pub use crate::{ reference::{Reference, ReferenceFlags, ReferenceId}, scope::ScopeTree, - symbol::SymbolTable, + symbol::{IsGlobalReference, SymbolTable}, }; /// Semantic analysis of a JavaScript/TypeScript program. diff --git a/crates/oxc_semantic/src/symbol.rs b/crates/oxc_semantic/src/symbol.rs index 991a925e9e655..fee5d6b00e01d 100644 --- a/crates/oxc_semantic/src/symbol.rs +++ b/crates/oxc_semantic/src/symbol.rs @@ -1,6 +1,6 @@ #![allow(non_snake_case)] // Silence erroneous warnings from Rust Analyser for `#[derive(Tsify)]` -use oxc_ast::ast::IdentifierReference; +use oxc_ast::ast::{Expression, IdentifierReference}; use oxc_index::IndexVec; use oxc_span::{CompactStr, Span}; pub use oxc_syntax::{ @@ -172,15 +172,6 @@ impl SymbolTable { self.references[reference_id].symbol_id().is_some() } - #[inline] - pub fn is_global_reference(&self, reference_id: ReferenceId) -> bool { - self.references[reference_id].symbol_id().is_none() - } - - pub fn is_global_identifier_reference(&self, ident: &IdentifierReference<'_>) -> bool { - ident.reference_id.get().is_some_and(|reference_id| self.is_global_reference(reference_id)) - } - #[inline] pub fn get_resolved_reference_ids(&self, symbol_id: SymbolId) -> &Vec { &self.resolved_references[symbol_id] @@ -217,3 +208,47 @@ impl SymbolTable { self.references.reserve(additional_references); } } + +/// Checks whether the a identifier reference is a global value or not. +pub trait IsGlobalReference { + fn is_global_reference(&self, _symbols: &SymbolTable) -> bool; + fn is_global_reference_name(&self, name: &str, _symbols: &SymbolTable) -> bool; +} + +impl IsGlobalReference for ReferenceId { + fn is_global_reference(&self, symbols: &SymbolTable) -> bool { + symbols.references[*self].symbol_id().is_none() + } + + fn is_global_reference_name(&self, _name: &str, _symbols: &SymbolTable) -> bool { + panic!("This function is pointless to be called."); + } +} + +impl<'a> IsGlobalReference for IdentifierReference<'a> { + fn is_global_reference(&self, symbols: &SymbolTable) -> bool { + self.reference_id + .get() + .is_some_and(|reference_id| reference_id.is_global_reference(symbols)) + } + + fn is_global_reference_name(&self, name: &str, symbols: &SymbolTable) -> bool { + self.name == name && self.is_global_reference(symbols) + } +} + +impl<'a> IsGlobalReference for Expression<'a> { + fn is_global_reference(&self, symbols: &SymbolTable) -> bool { + if let Expression::Identifier(ident) = self { + return ident.is_global_reference(symbols); + } + false + } + + fn is_global_reference_name(&self, name: &str, symbols: &SymbolTable) -> bool { + if let Expression::Identifier(ident) = self { + return ident.is_global_reference_name(name, symbols); + } + false + } +}