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

feat(semantic): move redeclare varaibles to symbol table #2614

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 14 additions & 20 deletions crates/oxc_linter/src/rules/eslint/no_redeclare.rs
Expand Up @@ -7,7 +7,6 @@ use oxc_diagnostics::{
thiserror::{self, Error},
};
use oxc_macros::declare_oxc_lint;
use oxc_semantic::VariableInfo;
use oxc_span::{CompactStr, Span};

use crate::{context::LintContext, rule::Rule};
Expand Down Expand Up @@ -73,23 +72,27 @@ impl Rule for NoRedeclare {
}

fn run_once(&self, ctx: &LintContext) {
let redeclare_variables = ctx.semantic().redeclare_variables();
let symbol_table = ctx.semantic().symbols();

for variable in redeclare_variables {
let decl = symbol_table.get_declaration(variable.symbol_id);
for symbol_id in ctx.symbols().iter() {
let decl = symbol_table.get_declaration(symbol_id);
let symbol_name = symbol_table.get_name(symbol_id);
match ctx.nodes().kind(decl) {
AstKind::VariableDeclarator(var) => {
if let BindingPatternKind::BindingIdentifier(ident) = &var.id.kind {
if symbol_table.get_name(variable.symbol_id) == ident.name.as_str() {
self.report_diagnostic(ctx, variable, ident);
if symbol_name == ident.name.as_str() {
for span in ctx.symbols().get_redeclare_variables(symbol_id) {
self.report_diagnostic(ctx, *span, ident);
}
}
}
}
AstKind::FormalParameter(param) => {
if let BindingPatternKind::BindingIdentifier(ident) = &param.pattern.kind {
if symbol_table.get_name(variable.symbol_id) == ident.name.as_str() {
self.report_diagnostic(ctx, variable, ident);
if symbol_name == ident.name.as_str() {
for span in ctx.symbols().get_redeclare_variables(symbol_id) {
self.report_diagnostic(ctx, *span, ident);
}
}
}
}
Expand All @@ -100,23 +103,14 @@ impl Rule for NoRedeclare {
}

impl NoRedeclare {
fn report_diagnostic(
&self,
ctx: &LintContext,
variable: &VariableInfo,
ident: &BindingIdentifier,
) {
fn report_diagnostic(&self, ctx: &LintContext, span: Span, ident: &BindingIdentifier) {
if self.built_in_globals && ctx.env_contains_var(&ident.name) {
ctx.diagnostic(NoRedeclareAsBuiltiInDiagnostic(
ident.name.to_compact_str(),
ident.span,
));
} else if variable.span != ident.span {
ctx.diagnostic(NoRedeclareDiagnostic(
ident.name.to_compact_str(),
ident.span,
variable.span,
));
} else {
ctx.diagnostic(NoRedeclareDiagnostic(ident.name.to_compact_str(), ident.span, span));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_semantic/src/binder.rs
Expand Up @@ -8,7 +8,7 @@ use oxc_ast::{
};
use oxc_span::{Atom, SourceType};

use crate::{scope::ScopeFlags, symbol::SymbolFlags, SemanticBuilder, VariableInfo};
use crate::{scope::ScopeFlags, symbol::SymbolFlags, SemanticBuilder};

pub trait Binder {
fn bind(&self, _builder: &mut SemanticBuilder) {}
Expand Down Expand Up @@ -59,7 +59,7 @@ impl<'a> Binder for VariableDeclarator<'a> {
builder.check_redeclaration(*scope_id, span, name, excludes, true)
{
ident.symbol_id.set(Some(symbol_id));
builder.add_redeclared_variables(VariableInfo { span: ident.span, symbol_id });
builder.add_redeclare_variable(symbol_id, ident.span);
return;
}
}
Expand Down
24 changes: 3 additions & 21 deletions crates/oxc_semantic/src/builder.rs
Expand Up @@ -29,17 +29,6 @@ use crate::{
Semantic,
};

#[derive(Debug, Clone)]
pub struct VariableInfo {
pub span: Span,
pub symbol_id: SymbolId,
}

#[derive(Debug)]
pub struct RedeclareVariables {
pub variables: Vec<VariableInfo>,
}

pub struct SemanticBuilder<'a> {
pub source_text: &'a str,

Expand Down Expand Up @@ -79,8 +68,6 @@ pub struct SemanticBuilder<'a> {

check_syntax_error: bool,

redeclare_variables: RedeclareVariables,

pub cfg: ControlFlowGraph,

pub class_table_builder: ClassTableBuilder,
Expand Down Expand Up @@ -117,7 +104,6 @@ impl<'a> SemanticBuilder<'a> {
label_builder: LabelBuilder::default(),
jsdoc: JSDocBuilder::new(source_text, &trivias),
check_syntax_error: false,
redeclare_variables: RedeclareVariables { variables: vec![] },
cfg: ControlFlowGraph::new(),
class_table_builder: ClassTableBuilder::new(),
}
Expand Down Expand Up @@ -177,7 +163,6 @@ impl<'a> SemanticBuilder<'a> {
module_record: Arc::clone(&self.module_record),
jsdoc: self.jsdoc.build(),
unused_labels: self.label_builder.unused_node_ids,
redeclare_variables: self.redeclare_variables.variables,
cfg: self.cfg,
};
SemanticBuilderReturn { semantic, errors: self.errors.into_inner() }
Expand All @@ -195,7 +180,6 @@ impl<'a> SemanticBuilder<'a> {
module_record: Arc::new(ModuleRecord::default()),
jsdoc: self.jsdoc.build(),
unused_labels: self.label_builder.unused_node_ids,
redeclare_variables: self.redeclare_variables.variables,
cfg: self.cfg,
}
}
Expand Down Expand Up @@ -254,9 +238,7 @@ impl<'a> SemanticBuilder<'a> {
) -> SymbolId {
if let Some(symbol_id) = self.check_redeclaration(scope_id, span, name, excludes, true) {
self.symbols.union_flag(symbol_id, includes);
if includes.is_function_scoped_declaration() {
self.add_redeclared_variables(VariableInfo { span, symbol_id });
}
self.add_redeclare_variable(symbol_id, span);
return symbol_id;
}

Expand Down Expand Up @@ -338,8 +320,8 @@ impl<'a> SemanticBuilder<'a> {
}
}

pub fn add_redeclared_variables(&mut self, variable: VariableInfo) {
self.redeclare_variables.variables.push(variable);
pub fn add_redeclare_variable(&mut self, symbol_id: SymbolId, span: Span) {
self.symbols.add_redeclare_variable(symbol_id, span);
}

fn add_export_flag_for_export_identifier(&mut self) {
Expand Down
7 changes: 0 additions & 7 deletions crates/oxc_semantic/src/lib.rs
Expand Up @@ -30,7 +30,6 @@ pub use oxc_syntax::{
use rustc_hash::FxHashSet;

pub use crate::{
builder::VariableInfo,
control_flow::{
print_basic_block, AssignmentValue, BasicBlockElement, BinaryAssignmentValue, BinaryOp,
CallType, CalleeWithArgumentsAssignmentValue, CollectionAssignmentValue, ControlFlowGraph,
Expand Down Expand Up @@ -64,8 +63,6 @@ pub struct Semantic<'a> {

unused_labels: FxHashSet<AstNodeId>,

redeclare_variables: Vec<VariableInfo>,

cfg: ControlFlowGraph,
}

Expand Down Expand Up @@ -147,10 +144,6 @@ impl<'a> Semantic<'a> {
pub fn is_reference_to_global_variable(&self, ident: &IdentifierReference) -> bool {
self.scopes().root_unresolved_references().contains_key(ident.name.as_str())
}

pub fn redeclare_variables(&self) -> &Vec<VariableInfo> {
&self.redeclare_variables
}
}

#[cfg(test)]
Expand Down
12 changes: 11 additions & 1 deletion crates/oxc_semantic/src/symbol.rs
Expand Up @@ -38,6 +38,7 @@ pub struct SymbolTable {
pub declarations: IndexVec<SymbolId, AstNodeId>,
pub resolved_references: IndexVec<SymbolId, Vec<ReferenceId>>,
pub references: IndexVec<ReferenceId, Reference>,
pub redeclare_variables: IndexVec<SymbolId, Vec<Span>>,
}

impl SymbolTable {
Expand Down Expand Up @@ -89,6 +90,10 @@ impl SymbolTable {
self.flags[symbol_id]
}

pub fn get_redeclare_variables(&self, symbol_id: SymbolId) -> &Vec<Span> {
&self.redeclare_variables[symbol_id]
}

pub fn union_flag(&mut self, symbol_id: SymbolId, includes: SymbolFlags) {
self.flags[symbol_id] |= includes;
}
Expand Down Expand Up @@ -120,13 +125,18 @@ impl SymbolTable {
_ = self.names.push(name.into_compact_str());
_ = self.flags.push(flag);
_ = self.scope_ids.push(scope_id);
self.resolved_references.push(vec![])
_ = self.resolved_references.push(vec![]);
self.redeclare_variables.push(vec![])
}

pub fn add_declaration(&mut self, node_id: AstNodeId) {
self.declarations.push(node_id);
}

pub fn add_redeclare_variable(&mut self, symbol_id: SymbolId, span: Span) {
self.redeclare_variables[symbol_id].push(span);
}

pub fn create_reference(&mut self, reference: Reference) -> ReferenceId {
self.references.push(reference)
}
Expand Down