Skip to content

Commit

Permalink
refactor(linter): use scope_id etc methods (#7394)
Browse files Browse the repository at this point in the history
Utilize the methods added in #7127 in `oxc_linter`.
  • Loading branch information
overlookmotel committed Nov 21, 2024
1 parent 224775c commit c34d649
Show file tree
Hide file tree
Showing 25 changed files with 80 additions and 141 deletions.
5 changes: 1 addition & 4 deletions crates/oxc_linter/src/ast_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,10 +295,7 @@ pub fn get_symbol_id_of_variable(
ident: &IdentifierReference,
semantic: &Semantic<'_>,
) -> Option<SymbolId> {
let symbol_table = semantic.symbols();
let reference_id = ident.reference_id.get()?;
let reference = symbol_table.get_reference(reference_id);
reference.symbol_id()
semantic.symbols().get_reference(ident.reference_id()).symbol_id()
}

pub fn extract_regex_flags<'a>(
Expand Down
11 changes: 5 additions & 6 deletions crates/oxc_linter/src/rules/eslint/func_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,10 +449,9 @@ impl Rule for FuncNames {
|name| {
// if this name shadows a variable in the outer scope **and** that name is referenced
// inside the function body, it is unsafe to add a name to this function
if ctx
.scopes()
.find_binding(func.scope_id.get().unwrap(), &name)
.map_or(false, |shadowed_var| {
if ctx.scopes().find_binding(func.scope_id(), &name).map_or(
false,
|shadowed_var| {
ctx.semantic().symbol_references(shadowed_var).any(
|reference| {
func.span.contains_inclusive(
Expand All @@ -463,8 +462,8 @@ impl Rule for FuncNames {
)
},
)
})
{
},
) {
return fixer.noop();
}

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/no_else_return.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ fn is_safe_from_name_collisions(

match stmt {
Statement::BlockStatement(block) => {
let block_scope_id = block.scope_id.get().unwrap();
let block_scope_id = block.scope_id();
let bindings = scopes.get_bindings(block_scope_id);
let parent_bindings = scopes.get_bindings(parent_scope_id);

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/no_import_assign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ fn is_argument_of_well_known_mutation_function(node_id: NodeId, ctx: &LintContex

if ((ident.name == "Object" && OBJECT_MUTATION_METHODS.contains(property_name))
|| (ident.name == "Reflect" && REFLECT_MUTATION_METHODS.contains(property_name)))
&& ident.reference_id.get().is_some_and(|id| !ctx.symbols().has_binding(id))
&& !ctx.symbols().has_binding(ident.reference_id())
{
return expr
.arguments
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl<'a> HasAnyUsedBinding<'a> for BindingPatternKind<'a> {

impl<'a> HasAnyUsedBinding<'a> for BindingIdentifier<'a> {
fn has_any_used_binding(&self, ctx: BindingContext<'_, 'a>) -> bool {
self.symbol_id.get().is_some_and(|symbol_id| ctx.has_usages(symbol_id))
ctx.has_usages(self.symbol_id())
}
}
impl<'a> HasAnyUsedBinding<'a> for ObjectPattern<'a> {
Expand Down
7 changes: 2 additions & 5 deletions crates/oxc_linter/src/rules/eslint/no_unused_vars/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,17 +244,14 @@ impl GetSpan for Symbol<'_, '_> {
impl<'a> PartialEq<IdentifierReference<'a>> for Symbol<'_, 'a> {
fn eq(&self, other: &IdentifierReference<'a>) -> bool {
// cheap: no resolved reference means its a global reference
let Some(reference_id) = other.reference_id.get() else {
return false;
};
let reference = self.symbols().get_reference(reference_id);
let reference = self.symbols().get_reference(other.reference_id());
reference.symbol_id().is_some_and(|symbol_id| self.id == symbol_id)
}
}

impl<'a> PartialEq<BindingIdentifier<'a>> for Symbol<'_, 'a> {
fn eq(&self, id: &BindingIdentifier<'a>) -> bool {
id.symbol_id.get().is_some_and(|id| self.id == id)
self.id == id.symbol_id()
}
}

Expand Down
11 changes: 5 additions & 6 deletions crates/oxc_linter/src/rules/eslint/no_unused_vars/usage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -731,21 +731,20 @@ impl<'s, 'a> Symbol<'s, 'a> {
for parent in self.iter_relevant_parents_of(node_id) {
match parent.kind() {
AstKind::Function(f) => {
return f.id.as_ref().and_then(|id| id.symbol_id.get());
return f.id.as_ref().map(BindingIdentifier::symbol_id);
}
AstKind::ArrowFunctionExpression(_) => {
needs_variable_identifier = true;
continue;
}
AstKind::VariableDeclarator(decl) if needs_variable_identifier => {
return decl.id.get_binding_identifier().and_then(|id| id.symbol_id.get());
return decl.id.get_binding_identifier().map(BindingIdentifier::symbol_id);
}
AstKind::AssignmentTarget(target) if needs_variable_identifier => {
return match target {
AssignmentTarget::AssignmentTargetIdentifier(id) => id
.reference_id
.get()
.and_then(|rid| self.symbols().get_reference(rid).symbol_id()),
AssignmentTarget::AssignmentTargetIdentifier(id) => {
self.symbols().get_reference(id.reference_id()).symbol_id()
}
_ => None,
};
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/no_var.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ fn is_written_to(binding_pat: &BindingPattern, ctx: &LintContext) -> bool {
match &binding_pat.kind {
BindingPatternKind::BindingIdentifier(binding_ident) => ctx
.semantic()
.symbol_references(binding_ident.symbol_id.get().expect("symbol id should be set"))
.symbol_references(binding_ident.symbol_id())
.any(oxc_semantic::Reference::is_write),
BindingPatternKind::ObjectPattern(object_pat) => {
if object_pat.properties.iter().any(|prop| is_written_to(&prop.value, ctx)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,12 @@ declare_oxc_lint!(
NoNamedAsDefaultMember,
suspicious
);

fn get_symbol_id_from_ident(
ctx: &LintContext<'_>,
ident: &IdentifierReference,
) -> Option<SymbolId> {
let reference_id = ident.reference_id.get().unwrap();
let reference = &ctx.symbols().references[reference_id];
reference.symbol_id()
ctx.symbols().get_reference(ident.reference_id()).symbol_id()
}

impl Rule for NoNamedAsDefaultMember {
Expand Down
4 changes: 1 addition & 3 deletions crates/oxc_linter/src/rules/jest/no_conditional_expect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,7 @@ fn check_parents<'a>(
return InConditional(false);
};
let symbol_table = ctx.semantic().symbols();
let Some(symbol_id) = ident.symbol_id.get() else {
return InConditional(false);
};
let symbol_id = ident.symbol_id();

// Consider cases like:
// ```javascript
Expand Down
3 changes: 1 addition & 2 deletions crates/oxc_linter/src/rules/jsdoc/require_returns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,7 @@ fn is_promise_resolve_with_value(expr: &Expression, ctx: &LintContext) -> Option
// })
// ```
// IMO: This is a fault of the original rule design...
for resolve_ref in ctx.symbols().get_resolved_references(ident.symbol_id.get()?)
{
for resolve_ref in ctx.symbols().get_resolved_references(ident.symbol_id()) {
// Check if `resolve` is called with value
match ctx.nodes().parent_node(resolve_ref.node_id())?.kind() {
// `resolve(foo)`
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/nextjs/inline_script_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl Rule for InlineScriptId {
}

'references_loop: for reference in
ctx.semantic().symbol_references(specifier.local.symbol_id.get().unwrap())
ctx.semantic().symbol_references(specifier.local.symbol_id())
{
let Some(node) = ctx.nodes().parent_node(reference.node_id()) else {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,7 @@ impl Rule for NoScriptComponentInHead {
return;
};

for reference in
ctx.semantic().symbol_references(default_import.local.symbol_id.get().unwrap())
{
for reference in ctx.semantic().symbol_references(default_import.local.symbol_id()) {
let Some(node) = ctx.nodes().parent_node(reference.node_id()) else {
return;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,7 @@ impl Rule for NoTitleInDocumentHead {
return;
};

for reference in
ctx.semantic().symbol_references(default_import.local.symbol_id.get().unwrap())
{
for reference in ctx.semantic().symbol_references(default_import.local.symbol_id()) {
let Some(node) = ctx.nodes().parent_node(reference.node_id()) else {
return;
};
Expand Down
7 changes: 2 additions & 5 deletions crates/oxc_linter/src/rules/oxc/no_accumulating_spread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,7 @@ impl Rule for NoAccumulatingSpread {
let symbols = ctx.semantic().symbols();

// get the AST node + symbol id of the declaration of the identifier
let Some(reference_id) = ident.reference_id.get() else {
return;
};
let reference = symbols.get_reference(reference_id);
let reference = symbols.get_reference(ident.reference_id());
let Some(referenced_symbol_id) = reference.symbol_id() else {
return;
};
Expand Down Expand Up @@ -305,7 +302,7 @@ fn get_reduce_diagnostic<'a>(

fn get_identifier_symbol_id(ident: &BindingPatternKind<'_>) -> Option<SymbolId> {
match ident {
BindingPatternKind::BindingIdentifier(ident) => ident.symbol_id.get(),
BindingPatternKind::BindingIdentifier(ident) => Some(ident.symbol_id()),
BindingPatternKind::AssignmentPattern(ident) => get_identifier_symbol_id(&ident.left.kind),
_ => None,
}
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/oxc/no_map_spread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ where
let v = Self {
ctx,
cb,
cb_scope_id: f.scope_id.get().unwrap(),
cb_scope_id: f.scope_id(),
is_in_return: f.expression,
return_span: f.expression.then(|| f.body.span()),
};
Expand All @@ -608,7 +608,7 @@ where
let v = Self {
ctx,
cb,
cb_scope_id: f.scope_id.get().unwrap(),
cb_scope_id: f.scope_id(),
is_in_return: false,
return_span: None,
};
Expand Down
54 changes: 16 additions & 38 deletions crates/oxc_linter/src/rules/oxc/only_used_in_recursion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,7 @@ fn create_diagnostic(
function_span: Span,
) {
let is_last_arg = arg_index == function_parameters.items.len() - 1;
let is_exported = ctx
.semantic()
.symbols()
.get_flags(function_id.symbol_id.get().expect("`symbol_id` should be set"))
.is_export();
let is_exported = ctx.semantic().symbols().get_flags(function_id.symbol_id()).is_export();

let is_diagnostic_only = !is_last_arg || is_exported;

Expand All @@ -153,26 +149,17 @@ fn create_diagnostic(
only_used_in_recursion_diagnostic(arg.span, arg.name.as_str()),
|fixer| {
let mut fix = fixer.new_fix_with_capacity(
ctx.semantic()
.symbol_references(arg.symbol_id.get().expect("`symbol_id` should be set"))
.count()
+ 1,
ctx.semantic().symbol_references(arg.symbol_id()).count() + 1,
);
fix.push(Fix::delete(arg.span()));

for reference in ctx
.semantic()
.symbol_references(arg.symbol_id.get().expect("`symbol_id` should be set"))
{
for reference in ctx.semantic().symbol_references(arg.symbol_id()) {
let node = ctx.nodes().get_node(reference.node_id());
fix.push(Fix::delete(node.span()));
}

// search for references to the function and remove the argument
for reference in ctx
.semantic()
.symbol_references(function_id.symbol_id.get().expect("`symbol_id` should be set"))
{
for reference in ctx.semantic().symbol_references(function_id.symbol_id()) {
let node = ctx.nodes().get_node(reference.node_id());

if let Some(AstKind::CallExpression(call_expr)) = ctx.nodes().parent_kind(node.id())
Expand Down Expand Up @@ -206,16 +193,15 @@ fn create_diagnostic_jsx(
function_id: &BindingIdentifier,
property: &BindingProperty,
) {
let Some(function_symbol_id) = function_id.symbol_id.get() else { return };
let is_exported = ctx.semantic().symbols().get_flags(function_symbol_id).is_export();
let is_exported = ctx.semantic().symbols().get_flags(function_id.symbol_id()).is_export();

let Some(property_name) = &property.key.static_name() else { return };
if is_exported {
return ctx.diagnostic(only_used_in_recursion_diagnostic(property.span(), property_name));
}

let Some(property_ident) = property.value.get_binding_identifier() else { return };
let Some(property_symbol_id) = property_ident.symbol_id.get() else { return };
let property_symbol_id = property_ident.symbol_id();
let mut references = ctx.semantic().symbol_references(property_symbol_id);

let has_spread_attribute = references.any(|x| used_with_spread_attribute(x.node_id(), ctx));
Expand Down Expand Up @@ -281,17 +267,14 @@ fn is_argument_only_used_in_recursion<'a>(
arg_index: usize,
ctx: &'a LintContext<'_>,
) -> bool {
let mut references = ctx
.semantic()
.symbol_references(arg.symbol_id.get().expect("`symbol_id` should be set"))
.peekable();
let mut references = ctx.semantic().symbol_references(arg.symbol_id()).peekable();

// Avoid returning true for an empty iterator
if references.peek().is_none() {
return false;
}

let function_symbol_id = function_id.symbol_id.get().unwrap();
let function_symbol_id = function_id.symbol_id();

for reference in references {
let Some(AstKind::Argument(argument)) = ctx.nodes().parent_kind(reference.node_id()) else {
Expand Down Expand Up @@ -325,15 +308,12 @@ fn is_property_only_used_in_recursion_jsx(
function_ident: &BindingIdentifier,
ctx: &LintContext,
) -> bool {
let Some(ident_symbol_id) = ident.symbol_id.get() else { return false };
let mut references = ctx.semantic().symbol_references(ident_symbol_id).peekable();

let mut references = ctx.semantic().symbol_references(ident.symbol_id()).peekable();
if references.peek().is_none() {
return false;
}

let Some(function_symbol_id) = function_ident.symbol_id.get() else { return false };

let function_symbol_id = function_ident.symbol_id();
for reference in references {
// Conditions:
// 1. The reference is inside a JSXExpressionContainer.
Expand Down Expand Up @@ -412,14 +392,12 @@ fn is_function_maybe_reassigned<'a>(
function_id: &'a BindingIdentifier,
ctx: &'a LintContext<'_>,
) -> bool {
ctx.semantic()
.symbol_references(function_id.symbol_id.get().expect("`symbol_id` should be set"))
.any(|reference| {
matches!(
ctx.nodes().parent_kind(reference.node_id()),
Some(AstKind::SimpleAssignmentTarget(_))
)
})
ctx.semantic().symbol_references(function_id.symbol_id()).any(|reference| {
matches!(
ctx.nodes().parent_kind(reference.node_id()),
Some(AstKind::SimpleAssignmentTarget(_))
)
})
}

fn get_jsx_element_symbol_id<'a>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,7 @@ impl Rule for ConsistentTypeImports {

if let Some(specifiers) = &import_decl.specifiers {
for specifier in specifiers {
let Some(symbol_id) = specifier.local().symbol_id.get() else {
continue;
};
let symbol_id = specifier.local().symbol_id();
let no_type_qualifier = match specifier {
ImportDeclarationSpecifier::ImportSpecifier(specifier) => {
specifier.import_kind.is_value()
Expand Down
5 changes: 1 addition & 4 deletions crates/oxc_linter/src/rules/typescript/prefer_for_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ impl Rule for PreferForOf {

let decl = &for_stmt_init.declarations[0];
let (var_name, var_symbol_id) = match &decl.id.kind {
BindingPatternKind::BindingIdentifier(id) => (&id.name, id.symbol_id.get()),
BindingPatternKind::BindingIdentifier(id) => (&id.name, id.symbol_id()),
_ => return,
};

Expand Down Expand Up @@ -183,9 +183,6 @@ impl Rule for PreferForOf {
let nodes = ctx.nodes();
let body_span = for_stmt.body.span();

let Some(var_symbol_id) = var_symbol_id else {
return;
};
if ctx.semantic().symbol_references(var_symbol_id).any(|reference| {
let ref_id = reference.node_id();

Expand Down
Loading

0 comments on commit c34d649

Please sign in to comment.