refactor(linter): use scope_id etc methods (#7394)

Utilize the methods added in #7127 in `oxc_linter`.
This commit is contained in:
overlookmotel 2024-11-21 12:29:57 +00:00
parent 224775c056
commit c34d649176
25 changed files with 80 additions and 141 deletions

View file

@ -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>(

View file

@ -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(
@ -463,8 +462,8 @@ impl Rule for FuncNames {
)
},
)
})
{
},
) {
return fixer.noop();
}

View file

@ -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);

View file

@ -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

View file

@ -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> {

View file

@ -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()
}
}

View file

@ -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,
};
}

View file

@ -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)) {

View file

@ -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 {

View file

@ -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

View file

@ -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)`

View file

@ -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;

View file

@ -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;
};

View file

@ -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;
};

View file

@ -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;
};
@ -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,
}

View file

@ -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()),
};
@ -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,
};

View file

@ -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;
@ -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())
@ -206,8 +193,7 @@ 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 {
@ -215,7 +201,7 @@ fn create_diagnostic_jsx(
}
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));
@ -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 {
@ -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.
@ -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>(

View file

@ -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()

View file

@ -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,
};
@ -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();

View file

@ -100,7 +100,7 @@ impl Rule for CatchErrorName {
}
if binding_ident.name.starts_with('_') {
if symbol_has_references(binding_ident.symbol_id.get(), ctx) {
if symbol_has_references(binding_ident.symbol_id(), ctx) {
ctx.diagnostic(catch_error_name_diagnostic(
binding_ident.name.as_str(),
&self.name,
@ -161,7 +161,7 @@ impl CatchErrorName {
}
if v.name.starts_with('_') {
if symbol_has_references(v.symbol_id.get(), ctx) {
if symbol_has_references(v.symbol_id(), ctx) {
ctx.diagnostic(catch_error_name_diagnostic(
v.name.as_str(),
&self.name,
@ -185,7 +185,7 @@ impl CatchErrorName {
}
if binding_ident.name.starts_with('_') {
if symbol_has_references(binding_ident.symbol_id.get(), ctx) {
if symbol_has_references(binding_ident.symbol_id(), ctx) {
ctx.diagnostic(catch_error_name_diagnostic(
binding_ident.name.as_str(),
&self.name,
@ -209,11 +209,8 @@ impl CatchErrorName {
}
}
fn symbol_has_references(symbol_id: Option<SymbolId>, ctx: &LintContext) -> bool {
if let Some(symbol_id) = symbol_id {
return ctx.semantic().symbol_references(symbol_id).next().is_some();
}
false
fn symbol_has_references(symbol_id: SymbolId, ctx: &LintContext) -> bool {
ctx.semantic().symbol_references(symbol_id).next().is_some()
}
#[test]

View file

@ -87,17 +87,13 @@ impl Rule for ConsistentExistenceIndexCheck {
return;
};
let Some(reference_id) = identifier.reference_id.get() else {
return;
};
let Some(reference_symbol_id) = ctx.symbols().get_reference(reference_id).symbol_id()
let Some(symbol_id) = ctx.symbols().get_reference(identifier.reference_id()).symbol_id()
else {
return;
};
let declaration = ctx.symbols().get_declaration(reference_symbol_id);
let node = ctx.nodes().get_node(declaration);
let declaration_node_id = ctx.symbols().get_declaration(symbol_id);
let node = ctx.nodes().get_node(declaration_node_id);
if let AstKind::VariableDeclarator(variables_declarator) = node.kind() {
if variables_declarator.kind != VariableDeclarationKind::Const {

View file

@ -166,15 +166,14 @@ impl Rule for ConsistentFunctionScoping {
return;
}
if let Some(func_scope_id) = function.scope_id.get() {
if let Some(parent_scope_id) = ctx.scopes().get_parent_id(func_scope_id) {
// Example: const foo = function bar() {};
// The bar function scope id is 1. In order to ignore this rule,
// its parent's scope id (in this case `foo`'s scope id is 0 and is equal to root scope id)
// should be considered.
if parent_scope_id == ctx.scopes().root_scope_id() {
return;
}
let func_scope_id = function.scope_id();
if let Some(parent_scope_id) = ctx.scopes().get_parent_id(func_scope_id) {
// Example: const foo = function bar() {};
// The bar function scope id is 1. In order to ignore this rule,
// its parent's scope id (in this case `foo`'s scope id is 0 and is equal to root scope id)
// should be considered.
if parent_scope_id == ctx.scopes().root_scope_id() {
return;
}
}
@ -184,7 +183,7 @@ impl Rule for ConsistentFunctionScoping {
if let Some(binding_ident) = get_function_like_declaration(node, ctx) {
(
binding_ident.symbol_id.get().unwrap(),
binding_ident.symbol_id(),
function_body,
function.id.as_ref().map_or(
Span::sized(function.span.start, 8),
@ -192,10 +191,7 @@ impl Rule for ConsistentFunctionScoping {
),
)
} else if let Some(function_id) = &function.id {
let Some(symbol_id) = function_id.symbol_id.get() else {
return;
};
(symbol_id, function_body, function_id.span())
(function_id.symbol_id(), function_body, function_id.span())
} else {
return;
}
@ -204,11 +200,8 @@ impl Rule for ConsistentFunctionScoping {
let Some(binding_ident) = get_function_like_declaration(node, ctx) else {
return;
};
let Some(symbol_id) = binding_ident.symbol_id.get() else {
return;
};
(symbol_id, &arrow_function.body, binding_ident.span())
(binding_ident.symbol_id(), &arrow_function.body, binding_ident.span())
}
_ => return,
};

View file

@ -235,23 +235,22 @@ fn check_expression(expr: &Expression, ctx: &LintContext<'_>) -> Option<oxc_span
lit.quasi().and_then(|quasi| if quasi == "then" { Some(lit.span) } else { None })
}
Expression::Identifier(ident) => {
let tab = ctx.semantic().symbols();
ident.reference_id.get().and_then(|ref_id| {
tab.get_reference(ref_id).symbol_id().and_then(|symbol_id| {
let decl = ctx.semantic().nodes().get_node(tab.get_declaration(symbol_id));
let var_decl = decl.kind().as_variable_declarator()?;
let symbols = ctx.semantic().symbols();
let reference_id = ident.reference_id();
symbols.get_reference(reference_id).symbol_id().and_then(|symbol_id| {
let decl = ctx.semantic().nodes().get_node(symbols.get_declaration(symbol_id));
let var_decl = decl.kind().as_variable_declarator()?;
match var_decl.init {
Some(Expression::StringLiteral(ref lit)) => {
if lit.value == "then" {
Some(lit.span)
} else {
None
}
match var_decl.init {
Some(Expression::StringLiteral(ref lit)) => {
if lit.value == "then" {
Some(lit.span)
} else {
None
}
_ => None,
}
})
_ => None,
}
})
}
_ => None,

View file

@ -85,7 +85,7 @@ impl Rule for PreferOptionalCatchBinding {
fn get_param_references_count(binding_pat: &BindingPattern, ctx: &LintContext) -> usize {
match &binding_pat.kind {
BindingPatternKind::BindingIdentifier(binding_ident) => {
ctx.semantic().symbol_references(binding_ident.symbol_id.get().unwrap()).count()
ctx.semantic().symbol_references(binding_ident.symbol_id()).count()
}
BindingPatternKind::ObjectPattern(object_pat) => {
let mut count = 0;

View file

@ -138,7 +138,7 @@ pub fn find_initialized_binding<'a, 'b>(
match &assignment.left.kind {
BindingPatternKind::BindingIdentifier(id) => {
// look for `x = {}`, or recurse if lhs is a binding pattern
if id.symbol_id.get().is_some_and(|binding_id| binding_id == symbol_id) {
if id.symbol_id() == symbol_id {
return Some((id.as_ref(), &assignment.right));
}
None