Skip to content

Commit

Permalink
feat: resolve correct symbol
Browse files Browse the repository at this point in the history
  • Loading branch information
Dunqing committed Jul 16, 2024
1 parent a4b087f commit c6c20c3
Show file tree
Hide file tree
Showing 15 changed files with 61 additions and 66 deletions.
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/eslint/no_global_assign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ impl Rule for NoGlobalAssign {

fn run_once(&self, ctx: &LintContext) {
let symbol_table = ctx.symbols();
for reference_id_list in ctx.scopes().root_unresolved_references().values() {
for &reference_id in reference_id_list {
for reference_id_list in ctx.scopes().root_unresolved_references_ids() {
for reference_id in reference_id_list {
let reference = symbol_table.get_reference(reference_id);
if reference.is_write() {
let name = reference.name();
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/eslint/no_undef.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ impl Rule for NoUndef {
fn run_once(&self, ctx: &LintContext) {
let symbol_table = ctx.symbols();

for reference_id_list in ctx.scopes().root_unresolved_references().values() {
for &reference_id in reference_id_list {
for reference_id_list in ctx.scopes().root_unresolved_references_ids() {
for reference_id in reference_id_list {
let reference = symbol_table.get_reference(reference_id);
let name = reference.name();

Expand Down
22 changes: 13 additions & 9 deletions crates/oxc_linter/src/rules/jest/no_confusing_set_timeout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,19 @@ impl Rule for NoConfusingSetTimeout {
let mut jest_reference_id_list: Vec<(ReferenceId, Span)> = vec![];
let mut seen_jest_set_timeout = false;

for reference_ids in scopes.root_unresolved_references().values() {
for reference_ids in scopes.root_unresolved_references_ids() {
collect_jest_reference_id(reference_ids, &mut jest_reference_id_list, ctx);
}

for reference_ids in &symbol_table.resolved_references {
collect_jest_reference_id(reference_ids, &mut jest_reference_id_list, ctx);
collect_jest_reference_id(
reference_ids.iter().copied(),
&mut jest_reference_id_list,
ctx,
);
}

for reference_id_list in scopes.root_unresolved_references().values() {
for reference_id_list in scopes.root_unresolved_references_ids() {
handle_jest_set_time_out(
ctx,
reference_id_list,
Expand All @@ -111,7 +115,7 @@ impl Rule for NoConfusingSetTimeout {
for reference_id_list in &symbol_table.resolved_references {
handle_jest_set_time_out(
ctx,
reference_id_list,
reference_id_list.iter().copied(),
&jest_reference_id_list,
&mut seen_jest_set_timeout,
&id_to_jest_node_map,
Expand All @@ -121,15 +125,15 @@ impl Rule for NoConfusingSetTimeout {
}

fn collect_jest_reference_id(
reference_id_list: &Vec<ReferenceId>,
reference_id_list: impl Iterator<Item = ReferenceId>,
jest_reference_list: &mut Vec<(ReferenceId, Span)>,
ctx: &LintContext,
) {
let symbol_table = ctx.symbols();
let nodes = ctx.nodes();

for reference_id in reference_id_list {
let reference = symbol_table.get_reference(*reference_id);
let reference = symbol_table.get_reference(reference_id);
if !is_jest_call(reference.name()) {
continue;
}
Expand All @@ -139,13 +143,13 @@ fn collect_jest_reference_id(
let AstKind::MemberExpression(member_expr) = parent_node.kind() else {
continue;
};
jest_reference_list.push((*reference_id, member_expr.span()));
jest_reference_list.push((reference_id, member_expr.span()));
}
}

fn handle_jest_set_time_out<'a>(
ctx: &LintContext<'a>,
reference_id_list: &Vec<ReferenceId>,
reference_id_list: impl Iterator<Item = ReferenceId>,
jest_reference_id_list: &Vec<(ReferenceId, Span)>,
seen_jest_set_timeout: &mut bool,
id_to_jest_node_map: &HashMap<AstNodeId, &PossibleJestNode<'a, '_>>,
Expand All @@ -154,7 +158,7 @@ fn handle_jest_set_time_out<'a>(
let scopes = ctx.scopes();
let symbol_table = ctx.symbols();

for &reference_id in reference_id_list {
for reference_id in reference_id_list {
let reference = symbol_table.get_reference(reference_id);

let Some(parent_node) = nodes.parent_node(reference.node_id()) else {
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/jest/no_jasmine_globals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl Rule for NoJasmineGlobals {
.filter(|(key, _)| NON_JASMINE_PROPERTY_NAMES.contains(&key.as_str()));

for (name, reference_ids) in jasmine_references {
for &reference_id in reference_ids {
for &(reference_id, _) in reference_ids {
let reference = symbol_table.get_reference(reference_id);
if let Some((error, help)) = get_non_jasmine_property_messages(name) {
ctx.diagnostic(no_jasmine_globals_diagnostic(error, help, reference.span()));
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/jest/no_mocks_import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl Rule for NoMocksImport {
return;
};

for reference_id in require_reference_ids {
for (reference_id, _) in require_reference_ids {
let reference = ctx.symbols().get_reference(*reference_id);
let Some(parent) = ctx.nodes().parent_node(reference.node_id()) else {
return;
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/nextjs/no_duplicate_head.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl Rule for NoDuplicateHead {
}

let flag = symbols.get_flag(symbol_id);
if !flag.is_import_binding() {
if !flag.is_import() {
return;
}

Expand Down
8 changes: 4 additions & 4 deletions crates/oxc_linter/src/utils/jest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use oxc_ast::{
},
AstKind,
};
use oxc_semantic::{AstNode, ReferenceId};
use oxc_semantic::{AstNode, ReferenceFlag, ReferenceId};
use phf::phf_set;

use crate::LintContext;
Expand Down Expand Up @@ -159,7 +159,7 @@ pub fn collect_possible_jest_call_node<'a, 'b>(
collect_ids_referenced_to_global(ctx)
.iter()
// set the original of global test function to None
.map(|id| (*id, None)),
.map(|(id, _)| (*id, None)),
);
}

Expand Down Expand Up @@ -236,13 +236,13 @@ fn find_original_name<'a>(import_decl: &'a ImportDeclaration<'a>, name: &str) ->
})
}

fn collect_ids_referenced_to_global(ctx: &LintContext) -> Vec<ReferenceId> {
fn collect_ids_referenced_to_global(ctx: &LintContext) -> Vec<(ReferenceId, ReferenceFlag)> {
ctx.scopes()
.root_unresolved_references()
.iter()
.filter(|(name, _)| JEST_METHOD_NAMES.contains(name.as_str()))
.flat_map(|(_, reference_ids)| reference_ids.clone())
.collect::<Vec<ReferenceId>>()
.collect::<Vec<(ReferenceId, ReferenceFlag)>>()
}

/// join name of the expression. e.g.
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_semantic/src/binder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ fn declare_symbol_for_import_specifier(
) {
SymbolFlags::TypeImport
} else {
SymbolFlags::ValueImport
SymbolFlags::Import
};

let symbol_id = builder.declare_symbol(
Expand Down
15 changes: 0 additions & 15 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,6 @@ impl<'a> SemanticBuilder<'a> {
let current_refs = iter.next().unwrap();

let bindings = self.scope.get_bindings(self.current_scope_id);
<<<<<<< HEAD
for (name, mut references) in current_refs.drain() {
// Try to resolve a reference.
// If unresolved, transfer it to parent scope's unresolved references.
Expand All @@ -404,7 +403,6 @@ impl<'a> SemanticBuilder<'a> {
Either::Right((ref_id, ref_flag))
}
});

if !resolved_reference_ids.is_empty() {
for reference_id in &resolved_reference_ids {
self.symbols.references[*reference_id].set_symbol_id(symbol_id);
Expand All @@ -419,19 +417,6 @@ impl<'a> SemanticBuilder<'a> {
}

if let Some(parent_reference_ids) = parent_refs.get_mut(&name) {
=======
for (name, references) in current_refs.drain() {
// Try to resolve a reference.
// If unresolved, transfer it to parent scope's unresolved references.
if let Some(symbol_id) = bindings.get(&name).copied() {
self.symbols.resolved_references[symbol_id].extend(references.iter().map(
|(reference_id, _)| {
self.symbols.references[*reference_id].set_symbol_id(symbol_id);
reference_id
},
));
} else if let Some(parent_reference_ids) = parent_refs.get_mut(&name) {
>>>>>>> eeb4351e6 (fix(semantic): resolve references to the incorrect symbol)
parent_reference_ids.extend(references);
} else {
parent_refs.insert(name, references);
Expand Down
38 changes: 19 additions & 19 deletions crates/oxc_semantic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,27 +254,27 @@ mod tests {
(SourceType::default(), "let a = { b: 1 }; a.b += 2", ReferenceFlag::read()),
// parens are pass-through
(SourceType::default(), "let a = 1, b; b = (a)", ReferenceFlag::read()),
(SourceType::default(), "let a = 1, b; b = ++(a)", ReferenceFlag::value()),
(SourceType::default(), "let a = 1, b; b = ++((((a))))", ReferenceFlag::value()),
(SourceType::default(), "let a = 1, b; b = ((++((a))))", ReferenceFlag::value()),
(SourceType::default(), "let a = 1, b; b = ++(a)", ReferenceFlag::read_write()),
(SourceType::default(), "let a = 1, b; b = ++((((a))))", ReferenceFlag::read_write()),
(SourceType::default(), "let a = 1, b; b = ((++((a))))", ReferenceFlag::read_write()),
// simple binops/calls for sanity check
(SourceType::default(), "let a, b; a + b", ReferenceFlag::read()),
(SourceType::default(), "let a, b; b(a)", ReferenceFlag::read()),
(SourceType::default(), "let a, b; a = 5", ReferenceFlag::write()),
// unary op counts as write, but checking continues up tree
(SourceType::default(), "let a = 1, b; b = ++a", ReferenceFlag::value()),
(SourceType::default(), "let a = 1, b; b = --a", ReferenceFlag::value()),
(SourceType::default(), "let a = 1, b; b = a++", ReferenceFlag::value()),
(SourceType::default(), "let a = 1, b; b = a--", ReferenceFlag::value()),
(SourceType::default(), "let a = 1, b; b = ++a", ReferenceFlag::read_write()),
(SourceType::default(), "let a = 1, b; b = --a", ReferenceFlag::read_write()),
(SourceType::default(), "let a = 1, b; b = a++", ReferenceFlag::read_write()),
(SourceType::default(), "let a = 1, b; b = a--", ReferenceFlag::read_write()),
// assignment expressions count as read-write
(SourceType::default(), "let a = 1, b; b = a += 5", ReferenceFlag::value()),
(SourceType::default(), "let a = 1; a += 5", ReferenceFlag::value()),
(SourceType::default(), "let a = 1, b; b = a += 5", ReferenceFlag::read_write()),
(SourceType::default(), "let a = 1; a += 5", ReferenceFlag::read_write()),
// note: we consider a to be written, and the read of `1` propagates upwards
(SourceType::default(), "let a, b; b = a = 1", ReferenceFlag::value()),
(SourceType::default(), "let a, b; b = (a = 1)", ReferenceFlag::value()),
(SourceType::default(), "let a, b; b = a = 1", ReferenceFlag::read_write()),
(SourceType::default(), "let a, b; b = (a = 1)", ReferenceFlag::read_write()),
(SourceType::default(), "let a, b, c; b = c = a", ReferenceFlag::read()),
// sequences return last value in sequence
(SourceType::default(), "let a, b; b = (0, a++)", ReferenceFlag::value()),
// sequences return last read_write in sequence
(SourceType::default(), "let a, b; b = (0, a++)", ReferenceFlag::read_write()),
// loops
(
SourceType::default(),
Expand All @@ -286,7 +286,7 @@ mod tests {
"var a, obj = { }; for(a of obj) { break }",
ReferenceFlag::write(),
),
(SourceType::default(), "var a; for(; false; a++) { }", ReferenceFlag::value()),
(SourceType::default(), "var a; for(; false; a++) { }", ReferenceFlag::read_write()),
(SourceType::default(), "var a = 1; while(a < 5) { break }", ReferenceFlag::read()),
// if statements
(SourceType::default(), "let a; if (a) { true } else { false }", ReferenceFlag::read()),
Expand All @@ -300,24 +300,24 @@ mod tests {
"let a, b; if (b == a) { true } else { false }",
ReferenceFlag::read(),
),
// identifiers not in last value are also considered a read (at
// identifiers not in last read_write are also considered a read (at
// least, or now)
(SourceType::default(), "let a, b; b = (a, 0)", ReferenceFlag::read()),
(SourceType::default(), "let a, b; b = (--a, 0)", ReferenceFlag::value()),
(SourceType::default(), "let a, b; b = (--a, 0)", ReferenceFlag::read_write()),
// other reads after a is written
// a = 1 writes, but the CallExpression reads the rhs (1) so a isn't read
(
SourceType::default(),
"let a; function foo(a) { return a }; foo(a = 1)",
ReferenceFlag::value(),
ReferenceFlag::read_write(),
),
// member expression
(SourceType::default(), "let a; a.b = 1", ReferenceFlag::read()),
(SourceType::default(), "let a; let b; b[a += 1] = 1", ReferenceFlag::value()),
(SourceType::default(), "let a; let b; b[a += 1] = 1", ReferenceFlag::read_write()),
(
SourceType::default(),
"let a; let b; let c; b[c[a = c['a']] = 'c'] = 'b'",
ReferenceFlag::value(),
ReferenceFlag::read_write(),
),
(
SourceType::default(),
Expand Down
8 changes: 7 additions & 1 deletion crates/oxc_semantic/src/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{symbol::SymbolId, AstNodeId};
type FxIndexMap<K, V> = IndexMap<K, V, BuildHasherDefault<FxHasher>>;

type Bindings = FxIndexMap<CompactStr, SymbolId>;
type UnresolvedReference = (ReferenceId, ReferenceFlag);
pub(crate) type UnresolvedReference = (ReferenceId, ReferenceFlag);
pub(crate) type UnresolvedReferences = FxHashMap<CompactStr, Vec<UnresolvedReference>>;

/// Scope Tree
Expand Down Expand Up @@ -92,6 +92,12 @@ impl ScopeTree {
&self.root_unresolved_references
}

pub fn root_unresolved_references_ids(
&self,
) -> impl Iterator<Item = impl Iterator<Item = ReferenceId> + '_> + '_ {
self.root_unresolved_references.values().map(|v| v.iter().map(|(id, _)| *id))
}

pub fn get_flags(&self, scope_id: ScopeId) -> ScopeFlags {
self.flags[scope_id]
}
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_semantic/tests/integration/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,11 @@ fn test_export_in_invalid_scope() {
fn test_import_assignment() {
SemanticTester::ts("import Foo = require('./foo')")
.has_root_symbol("Foo")
.contains_flags(SymbolFlags::ValueImport)
.contains_flags(SymbolFlags::Import)
.test();

SemanticTester::ts("import { Foo } from './foo'; import Baz = Foo.Bar.Baz")
.has_root_symbol("Baz")
.contains_flags(SymbolFlags::ValueImport)
.contains_flags(SymbolFlags::Import)
.test();
}
2 changes: 1 addition & 1 deletion crates/oxc_syntax/src/reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl ReferenceFlag {
Self::Write
}

pub const fn value() -> Self {
pub const fn read_write() -> Self {
Self::Value
}

Expand Down
12 changes: 6 additions & 6 deletions crates/oxc_syntax/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ bitflags! {
const Class = 1 << 5;
const CatchVariable = 1 << 6; // try {} catch(catch_variable) {}
const Function = 1 << 7;
const ValueImport = 1 << 8; // Imported ESM binding
const TypeImport = 1 << 9; // Imported ESM type binding
const Import = 1 << 8; // Imported ESM binding
const TypeImport = 1 << 9; // Imported ESM type-only binding
// Type specific symbol flags
const TypeAlias = 1 << 10;
const Interface = 1 << 11;
Expand Down Expand Up @@ -76,7 +76,7 @@ bitflags! {
const BlockScopedVariableExcludes = Self::Value.bits();

const ClassExcludes = (Self::Value.bits() | Self::TypeAlias.bits()) & !Self::ValueModule.bits() ;
const ImportBindingExcludes = Self::ValueImport.bits() | Self::TypeImport.bits();
const ImportBindingExcludes = Self::Import.bits() | Self::TypeImport.bits();
// Type specific excludes
const TypeAliasExcludes = Self::Type.bits();
const InterfaceExcludes = Self::Type.bits() & !(Self::Interface.bits() | Self::Class.bits());
Expand All @@ -95,11 +95,11 @@ impl SymbolFlags {
}

pub fn is_type(&self) -> bool {
self.intersects(Self::Type | Self::TypeImport)
self.intersects(Self::Type | Self::TypeImport | Self::Import)
}

pub fn is_value(&self) -> bool {
self.intersects(Self::Value | Self::ValueImport)
self.intersects(Self::Value | Self::Import | Self::Function)
}

pub fn is_const_variable(&self) -> bool {
Expand Down Expand Up @@ -127,6 +127,6 @@ impl SymbolFlags {
}

pub fn is_import(&self) -> bool {
self.contains(Self::ValueImport | Self::TypeImport)
self.intersects(Self::Import | Self::TypeImport)
}
}
2 changes: 1 addition & 1 deletion crates/oxc_traverse/src/context/scoping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ impl TraverseScoping {
flag: ReferenceFlag,
) -> ReferenceId {
let reference = Reference::new(SPAN, name.clone(), AstNodeId::dummy(), flag);
let reference_id = self.symbols.create_reference(reference.clone());
let reference_id = self.symbols.create_reference(reference);
self.scopes.add_root_unresolved_reference(name, (reference_id, flag));
reference_id
}
Expand Down

0 comments on commit c6c20c3

Please sign in to comment.