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

refactor(linter): use scope_id etc methods #7394

Merged
merged 1 commit into from
Nov 21, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading