diff --git a/crates/oxc_linter/src/rules/eslint/func_names.rs b/crates/oxc_linter/src/rules/eslint/func_names.rs index 27f51314e862f4..44b3b6badca73b 100644 --- a/crates/oxc_linter/src/rules/eslint/func_names.rs +++ b/crates/oxc_linter/src/rules/eslint/func_names.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use oxc_ast::ast::{ AssignmentTarget, AssignmentTargetProperty, BindingPatternKind, Expression, Function, FunctionType, MethodDefinitionKind, PropertyKey, PropertyKind, @@ -15,7 +17,7 @@ pub struct FuncNames { generators_config: FuncNamesConfig, } -#[derive(Debug, Default, Clone, PartialEq)] +#[derive(Debug, Default, Clone, Copy, PartialEq)] enum FuncNamesConfig { #[default] Always, @@ -23,83 +25,101 @@ enum FuncNamesConfig { Never, } +impl FuncNamesConfig { + fn is_invalid_function(self, func: &Function, parent_node: &AstNode<'_>) -> bool { + let func_name = get_function_name(func); + + match self { + Self::Never => func_name.is_some() && func.r#type != FunctionType::FunctionDeclaration, + Self::AsNeeded => func_name.is_none() && !has_inferred_name(func, parent_node), + Self::Always => func_name.is_none() && !is_object_or_class_method(parent_node), + } + } +} + impl TryFrom<&serde_json::Value> for FuncNamesConfig { type Error = OxcDiagnostic; fn try_from(raw: &serde_json::Value) -> Result { - if !raw.is_string() { - return Err(OxcDiagnostic::warn(format!( - "Expecting string for eslint/func-names configuration, got {raw}" - ))); - } - - match raw.as_str().unwrap() { - "always" => Ok(FuncNamesConfig::Always), - "as-needed" => Ok(FuncNamesConfig::AsNeeded), - "never" => Ok(FuncNamesConfig::Never), - v => Err(OxcDiagnostic::warn(format!( - "Expecting always, as-needed or never for eslint/func-names configuration, got {v}" - ))), - } + raw.as_str().map_or_else( + || Err(OxcDiagnostic::warn("Expecting string for eslint/func-names configuration")), + |v| match v { + "always" => Ok(FuncNamesConfig::Always), + "as-needed" => Ok(FuncNamesConfig::AsNeeded), + "never" => Ok(FuncNamesConfig::Never), + _ => Err(OxcDiagnostic::warn( + "Expecting always, as-needed or never for eslint/func-names configuration", + )), + }, + ) } } declare_oxc_lint!( /// ### What it does /// - /// Require or disallow named function expressions + /// Require or disallow named function expressions. /// /// ### Why is this bad? /// - /// Leaving the name off a function will cause `` to appear - /// in stack traces of errors thrown in it or any function called within it. - /// This makes it more difficult to find where an error is thrown. - /// If you provide the optional name for a function expression - /// then you will get the name of the function expression in the stack trace. + /// Leaving the name off a function will cause `` to appear in + /// stack traces of errors thrown in it or any function called within it. + /// This makes it more difficult to find where an error is thrown. If you + /// provide the optional name for a function expression then you will get + /// the name of the function expression in the stack trace. + /// + /// ## Configuration + /// This rule has a string option: + /// - `"always"` requires a function expression to have a name under all + /// circumstances. + /// - `"as-needed"` requires a function expression to have a name only when + /// one will not be automatically inferred by the runtime. + /// - `"never"` requires a function expression to not have a name under any + /// circumstances. /// - /// /// ### Example + /// ### Example /// - /// Example of **incorrect** code for this rule: + /// Examples of **incorrect** code for this rule: /// /// ```javascript - /// /*eslint func-names: "error" */ + /// /*oxlint func-names: "error" */ /// /// // default is "always" and there is an anonymous function /// Foo.prototype.bar = function() {}; /// - /// /*eslint func-names: ["error", "always"] */ + /// /*oxlint func-names: ["error", "always"] */ /// /// // there is an anonymous function /// Foo.prototype.bar = function() {}; /// - /// /*eslint func-names: ["error", "as-needed"] */ + /// /*oxlint func-names: ["error", "as-needed"] */ /// /// // there is an anonymous function /// // where the name isn’t assigned automatically per the ECMAScript specs /// Foo.prototype.bar = function() {}; /// - /// /*eslint func-names: ["error", "never"] */ + /// /*oxlint func-names: ["error", "never"] */ /// /// // there is a named function /// Foo.prototype.bar = function bar() {}; /// ``` /// - /// Example of **correct* code for this rule: + /// Examples of **correct* code for this rule: /// /// ```javascript - /// /*eslint func-names: "error" */ + /// /*oxlint func-names: "error" */ /// /// Foo.prototype.bar = function bar() {}; /// - /// /*eslint func-names: ["error", "always"] */ + /// /*oxlint func-names: ["error", "always"] */ /// /// Foo.prototype.bar = function bar() {}; /// - /// /*eslint func-names: ["error", "as-needed"] */ + /// /*oxlint func-names: ["error", "as-needed"] */ /// /// var foo = function(){}; /// - /// /*eslint func-names: ["error", "never"] */ + /// /*oxlint func-names: ["error", "never"] */ /// /// Foo.prototype.bar = function() {}; /// ``` @@ -107,40 +127,29 @@ declare_oxc_lint!( style, pending ); -/** - * Determines whether the current FunctionExpression node is a get, set, or - * shorthand method in an object literal or a class. - */ -fn is_object_or_class_method(parent_node: Option<&AstNode>) -> bool { - if parent_node.is_none() { - return false; - } - - let unwrapped_kind = parent_node.unwrap().kind(); - if matches!(unwrapped_kind, AstKind::MethodDefinition(_)) { - return true; - } - - if let AstKind::ObjectProperty(property) = unwrapped_kind { - return property.method - || property.kind == PropertyKind::Get - || property.kind == PropertyKind::Set; +/// Determines whether the current FunctionExpression node is a get, set, or +/// shorthand method in an object literal or a class. +fn is_object_or_class_method(parent_node: &AstNode) -> bool { + match parent_node.kind() { + AstKind::MethodDefinition(_) => true, + AstKind::ObjectProperty(property) => { + property.method + || property.kind == PropertyKind::Get + || property.kind == PropertyKind::Set + } + _ => false, } - - false } -/** - * Determines whether the current FunctionExpression node has a name that would be - * inferred from context in a conforming ES6 environment. - */ -fn has_inferred_name(function: &Function, parent_node: Option<&AstNode>) -> bool { + +/// Determines whether the current FunctionExpression node has a name that would be +/// inferred from context in a conforming ES6 environment. +fn has_inferred_name<'a>(function: &Function<'a>, parent_node: &AstNode<'a>) -> bool { if is_object_or_class_method(parent_node) { return true; } - // unwrap is safe because of is_object_or_class_method - match parent_node.unwrap().kind() { + match parent_node.kind() { AstKind::VariableDeclarator(declarator) => { matches!(declarator.id.kind, BindingPatternKind::BindingIdentifier(_)) && matches!(declarator.init.as_ref().unwrap(), Expression::FunctionExpression(function_expression) @@ -177,11 +186,18 @@ fn has_inferred_name(function: &Function, parent_node: Option<&AstNode>) -> bool } AstKind::ObjectAssignmentTarget(target) => { for property in &target.properties { - if matches!(property, AssignmentTargetProperty::AssignmentTargetPropertyIdentifier(identifier) - if matches!(identifier.init.as_ref().unwrap(), Expression::FunctionExpression(function_expression) - if get_function_identifier(function_expression) == get_function_identifier(function) - ) - ) { + let AssignmentTargetProperty::AssignmentTargetPropertyIdentifier(identifier) = + property + else { + continue; + }; + let Expression::FunctionExpression(function_expression) = + &identifier.init.as_ref().unwrap() + else { + continue; + }; + if get_function_identifier(function_expression) == get_function_identifier(function) + { return true; } } @@ -202,24 +218,24 @@ fn get_function_identifier<'a>(func: &'a Function<'a>) -> Option<&'a Span> { /** * Gets the identifier name of the function */ -fn get_function_name<'a>(func: &'a Function<'a>) -> Option<&Atom<'a>> { +fn get_function_name<'f, 'a>(func: &'f Function<'a>) -> Option<&'f Atom<'a>> { func.id.as_ref().map(|id| &id.name) } -fn get_property_key_name<'a>(key: &'a PropertyKey<'a>) -> Option { +fn get_property_key_name<'a>(key: &PropertyKey<'a>) -> Option> { if matches!(key, PropertyKey::NullLiteral(_)) { - return Some("null".to_string()); + return Some("null".into()); } match key { PropertyKey::RegExpLiteral(regex) => { - Some(format!("/{}/{}", regex.regex.pattern, regex.regex.flags)) + Some(Cow::Owned(format!("/{}/{}", regex.regex.pattern, regex.regex.flags))) } - PropertyKey::BigIntLiteral(bigint) => Some(bigint.raw.to_string()), + PropertyKey::BigIntLiteral(bigint) => Some(Cow::Borrowed(bigint.raw.as_str())), PropertyKey::TemplateLiteral(template) => { if template.expressions.len() == 0 && template.quasis.len() == 1 { if let Some(cooked) = &template.quasis[0].value.cooked { - return Some(cooked.to_string()); + return Some(Cow::Borrowed(cooked.as_str())); } } @@ -229,141 +245,98 @@ fn get_property_key_name<'a>(key: &'a PropertyKey<'a>) -> Option { } } -fn get_static_property_name<'a>(parent_node: Option<&'a AstNode<'a>>) -> Option { - parent_node?; - - let result_key = match parent_node.unwrap().kind() { - AstKind::PropertyDefinition(definition) => Some((&definition.key, definition.computed)), +fn get_static_property_name<'a>(parent_node: &AstNode<'a>) -> Option> { + let (key, computed) = match parent_node.kind() { + AstKind::PropertyDefinition(definition) => (&definition.key, definition.computed), AstKind::MethodDefinition(method_definition) => { - Some((&method_definition.key, method_definition.computed)) + (&method_definition.key, method_definition.computed) } - AstKind::ObjectProperty(property) => Some((&property.key, property.computed)), - _ => None, + AstKind::ObjectProperty(property) => (&property.key, property.computed), + _ => return None, }; - result_key?; - - let prop = result_key.unwrap().0; - - if prop.is_identifier() && !result_key.unwrap().1 { - prop.name()?; - - return Some(prop.name().unwrap().to_string()); + if key.is_identifier() && !computed { + return key.name(); } - get_property_key_name(prop) + get_property_key_name(key) } -/** - * Gets the name and kind of the given function node. - * @see - */ -fn get_function_name_with_kind(func: &Function, parent_node: Option<&AstNode>) -> String { - let mut tokens: Vec = vec![]; - - if parent_node.is_some() { - match parent_node.unwrap().kind() { - AstKind::MethodDefinition(definition) => { - if definition.r#static { - tokens.push("static".to_owned()); - } +/// Gets the name and kind of the given function node. +/// @see +fn get_function_name_with_kind<'a>(func: &Function<'a>, parent_node: &AstNode<'a>) -> Cow<'a, str> { + let mut tokens: Vec> = vec![]; - if !definition.computed && definition.key.is_private_identifier() { - tokens.push("private".to_owned()); - } + match parent_node.kind() { + AstKind::MethodDefinition(definition) => { + if definition.r#static { + tokens.push("static".into()); } - AstKind::PropertyDefinition(definition) => { - if definition.r#static { - tokens.push("static".to_owned()); - } - if !definition.computed && definition.key.is_private_identifier() { - tokens.push("private".to_owned()); - } + if !definition.computed && definition.key.is_private_identifier() { + tokens.push("private".into()); } - _ => {} } + AstKind::PropertyDefinition(definition) => { + if definition.r#static { + tokens.push("static".into()); + } + + if !definition.computed && definition.key.is_private_identifier() { + tokens.push("private".into()); + } + } + _ => {} } if func.r#async { - tokens.push("async".to_owned()); + tokens.push("async".into()); } if func.generator { - tokens.push("generator".to_owned()); + tokens.push("generator".into()); } - if parent_node.is_some() { - let kind = parent_node.unwrap().kind(); - - match kind { - AstKind::MethodDefinition(method_definition) => match method_definition.kind { - MethodDefinitionKind::Constructor => tokens.push("constructor".to_owned()), - MethodDefinitionKind::Get => tokens.push("getter".to_owned()), - MethodDefinitionKind::Set => tokens.push("setter".to_owned()), - MethodDefinitionKind::Method => tokens.push("method".to_owned()), - }, - AstKind::PropertyDefinition(_) => tokens.push("method".to_owned()), - _ => tokens.push("function".to_owned()), - } + match parent_node.kind() { + AstKind::MethodDefinition(method_definition) => match method_definition.kind { + MethodDefinitionKind::Constructor => tokens.push("constructor".into()), + MethodDefinitionKind::Get => tokens.push("getter".into()), + MethodDefinitionKind::Set => tokens.push("setter".into()), + MethodDefinitionKind::Method => tokens.push("method".into()), + }, + AstKind::PropertyDefinition(_) => tokens.push("method".into()), + _ => tokens.push("function".into()), + } - match kind { - AstKind::MethodDefinition(method_definition) - if !method_definition.computed && method_definition.key.is_private_identifier() => - { - if let Some(name) = method_definition.key.name() { - tokens.push(name.to_string()); - } + match parent_node.kind() { + AstKind::MethodDefinition(method_definition) + if !method_definition.computed && method_definition.key.is_private_identifier() => + { + if let Some(name) = method_definition.key.name() { + tokens.push(name); } - AstKind::PropertyDefinition(definition) => { - if !definition.computed && definition.key.is_private_identifier() { - if let Some(name) = definition.key.name() { - tokens.push(name.to_string()); - } - } else if let Some(static_name) = get_static_property_name(parent_node) { - tokens.push(static_name); - } else if let Some(name) = get_function_name(func) { - tokens.push(name.to_string()); + } + AstKind::PropertyDefinition(definition) => { + if !definition.computed && definition.key.is_private_identifier() { + if let Some(name) = definition.key.name() { + tokens.push(name); } + } else if let Some(static_name) = get_static_property_name(parent_node) { + tokens.push(static_name); + } else if let Some(name) = get_function_name(func) { + tokens.push(Cow::Borrowed(name.as_str())); } - _ => { - if let Some(static_name) = get_static_property_name(parent_node) { - tokens.push(static_name); - } else if let Some(name) = get_function_name(func) { - tokens.push(name.to_string()); - } + } + _ => { + if let Some(static_name) = get_static_property_name(parent_node) { + tokens.push(static_name); + } else if let Some(name) = get_function_name(func) { + tokens.push(Cow::Borrowed(name.as_str())); } } } - tokens.join(" ") -} - -fn is_invalid_function( - func: &Function, - config: &FuncNamesConfig, - parent_node: Option<&AstNode>, -) -> bool { - let func_name = get_function_name(func); - - match *config { - FuncNamesConfig::Never - if func_name.is_some() && func.r#type != FunctionType::FunctionDeclaration => - { - true - } - FuncNamesConfig::AsNeeded - if func_name.is_none() && !has_inferred_name(func, parent_node) => - { - true - } - FuncNamesConfig::Always - if func_name.is_none() && !is_object_or_class_method(parent_node) => - { - true - } - _ => false, - } + Cow::Owned(tokens.join(" ")) } impl Rule for FuncNames { @@ -384,18 +357,21 @@ impl Rule for FuncNames { Self { default_config, generators_config } } + fn run_once(&self, ctx: &LintContext<'_>) { - let mut invalid_funcs: Vec<(&Function, Option<&AstNode>)> = vec![]; + let mut invalid_funcs: Vec<(&Function, &AstNode)> = vec![]; for node in ctx.nodes().iter() { match node.kind() { // check function if it invalid, do not report it because maybe later the function is calling itself AstKind::Function(func) => { - let parent_node = ctx.nodes().parent_node(node.id()); + let Some(parent_node) = ctx.nodes().parent_node(node.id()) else { + continue; + }; let config = if func.generator { &self.generators_config } else { &self.default_config }; - if is_invalid_function(func, config, parent_node) { + if config.is_invalid_function(func, parent_node) { invalid_funcs.push((func, parent_node)); } } @@ -407,7 +383,8 @@ impl Rule for FuncNames { // check at first if the callee calls an invalid function if !invalid_funcs .iter() - .any(|(func, _)| get_function_name(func) == Some(&identifier.name)) + .filter_map(|(func, _)| get_function_name(func)) + .any(|func_name| func_name == &identifier.name) { continue; } @@ -442,7 +419,7 @@ impl Rule for FuncNames { for (func, parent_node) in &invalid_funcs { let func_name = get_function_name(func); - let func_name_complete = get_function_name_with_kind(func, *parent_node); + let func_name_complete = get_function_name_with_kind(func, parent_node); if func_name.is_some() { ctx.diagnostic(