diff --git a/crates/oxc_linter/src/ast_util.rs b/crates/oxc_linter/src/ast_util.rs index 9e535c72b654e..06a80753fbf6f 100644 --- a/crates/oxc_linter/src/ast_util.rs +++ b/crates/oxc_linter/src/ast_util.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use oxc_ast::{ ast::{BindingIdentifier, *}, AstKind, @@ -770,3 +772,129 @@ pub fn is_default_this_binding<'a>( } } } + +pub 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) => { + (&method_definition.key, method_definition.computed) + } + AstKind::ObjectProperty(property) => (&property.key, property.computed), + _ => return None, + }; + + if key.is_identifier() && !computed { + return key.name(); + } + + if matches!(key, PropertyKey::NullLiteral(_)) { + return Some("null".into()); + } + + match key { + PropertyKey::RegExpLiteral(regex) => { + Some(Cow::Owned(format!("/{}/{}", regex.regex.pattern, regex.regex.flags))) + } + 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(Cow::Borrowed(cooked.as_str())); + } + } + + None + } + _ => None, + } +} + +/// Gets the name and kind of the given function node. +/// @see +pub fn get_function_name_with_kind<'a>( + node: &AstNode<'a>, + parent_node: &AstNode<'a>, +) -> Cow<'a, str> { + let (name, is_async, is_generator) = match node.kind() { + AstKind::Function(func) => (func.name(), func.r#async, func.generator), + AstKind::ArrowFunctionExpression(arrow_func) => (None, arrow_func.r#async, false), + _ => (None, false, false), + }; + + let mut tokens: Vec> = vec![]; + + match parent_node.kind() { + AstKind::MethodDefinition(definition) => { + if !definition.computed && definition.key.is_private_identifier() { + tokens.push(Cow::Borrowed("private")); + } else if let Some(accessibility) = definition.accessibility { + tokens.push(Cow::Borrowed(accessibility.as_str())); + } + + if definition.r#static { + tokens.push(Cow::Borrowed("static")); + } + } + AstKind::PropertyDefinition(definition) => { + if !definition.computed && definition.key.is_private_identifier() { + tokens.push(Cow::Borrowed("private")); + } else if let Some(accessibility) = definition.accessibility { + tokens.push(Cow::Borrowed(accessibility.as_str())); + } + + if definition.r#static { + tokens.push(Cow::Borrowed("static")); + } + } + _ => {} + } + + if is_async { + tokens.push(Cow::Borrowed("async")); + } + + if is_generator { + tokens.push(Cow::Borrowed("generator")); + } + + match parent_node.kind() { + AstKind::MethodDefinition(method_definition) => match method_definition.kind { + MethodDefinitionKind::Constructor => tokens.push(Cow::Borrowed("constructor")), + MethodDefinitionKind::Get => tokens.push(Cow::Borrowed("getter")), + MethodDefinitionKind::Set => tokens.push(Cow::Borrowed("setter")), + MethodDefinitionKind::Method => tokens.push(Cow::Borrowed("method")), + }, + AstKind::PropertyDefinition(_) => tokens.push(Cow::Borrowed("method")), + _ => tokens.push(Cow::Borrowed("function")), + } + + 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); + } + } else if let Some(static_name) = get_static_property_name(parent_node) { + tokens.push(static_name); + } else if let Some(name) = name { + 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) = name { + tokens.push(Cow::Borrowed(name.as_str())); + } + } + } + + Cow::Owned(tokens.join(" ")) +} diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index b4a9458114e67..ce127e6756ecc 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -46,6 +46,7 @@ mod eslint { pub mod init_declarations; pub mod max_classes_per_file; pub mod max_lines; + pub mod max_lines_per_function; pub mod max_nested_callbacks; pub mod max_params; pub mod new_cap; @@ -554,6 +555,7 @@ oxc_macros::declare_all_lint_rules! { eslint::guard_for_in, eslint::init_declarations, eslint::max_nested_callbacks, + eslint::max_lines_per_function, eslint::max_classes_per_file, eslint::max_lines, eslint::max_params, diff --git a/crates/oxc_linter/src/rules/eslint/func_names.rs b/crates/oxc_linter/src/rules/eslint/func_names.rs index 4dc46f9022912..4b508fef0c0c4 100644 --- a/crates/oxc_linter/src/rules/eslint/func_names.rs +++ b/crates/oxc_linter/src/rules/eslint/func_names.rs @@ -3,7 +3,7 @@ use std::borrow::Cow; use oxc_ast::{ ast::{ AssignmentTarget, AssignmentTargetProperty, BindingPatternKind, Expression, Function, - FunctionType, MethodDefinitionKind, PropertyKey, PropertyKind, + FunctionType, PropertyKind, }, AstKind, }; @@ -14,7 +14,7 @@ use oxc_span::{Atom, GetSpan, Span}; use oxc_syntax::identifier::is_identifier_name; use phf::phf_set; -use crate::{context::LintContext, rule::Rule, AstNode}; +use crate::{ast_util::get_function_name_with_kind, context::LintContext, rule::Rule, AstNode}; fn named_diagnostic(function_name: &str, span: Span) -> OxcDiagnostic { OxcDiagnostic::warn(format!("Unexpected named {function_name}.")) @@ -233,127 +233,6 @@ fn get_function_identifier<'a>(func: &'a Function<'a>) -> Option<&'a Span> { func.id.as_ref().map(|id| &id.span) } -fn get_property_key_name<'a>(key: &PropertyKey<'a>) -> Option> { - if matches!(key, PropertyKey::NullLiteral(_)) { - return Some("null".into()); - } - - match key { - PropertyKey::RegExpLiteral(regex) => { - Some(Cow::Owned(format!("/{}/{}", regex.regex.pattern, regex.regex.flags))) - } - 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(Cow::Borrowed(cooked.as_str())); - } - } - - None - } - _ => None, - } -} - -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) => { - (&method_definition.key, method_definition.computed) - } - AstKind::ObjectProperty(property) => (&property.key, property.computed), - _ => return None, - }; - - if key.is_identifier() && !computed { - return key.name(); - } - - get_property_key_name(key) -} - -/// 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![]; - - match parent_node.kind() { - AstKind::MethodDefinition(definition) => { - if !definition.computed && definition.key.is_private_identifier() { - tokens.push(Cow::Borrowed("private")); - } else if let Some(accessibility) = definition.accessibility { - tokens.push(Cow::Borrowed(accessibility.as_str())); - } - - if definition.r#static { - tokens.push(Cow::Borrowed("static")); - } - } - AstKind::PropertyDefinition(definition) => { - if !definition.computed && definition.key.is_private_identifier() { - tokens.push(Cow::Borrowed("private")); - } else if let Some(accessibility) = definition.accessibility { - tokens.push(Cow::Borrowed(accessibility.as_str())); - } - - if definition.r#static { - tokens.push(Cow::Borrowed("static")); - } - } - _ => {} - } - - if func.r#async { - tokens.push(Cow::Borrowed("async")); - } - - if func.generator { - tokens.push(Cow::Borrowed("generator")); - } - - match parent_node.kind() { - AstKind::MethodDefinition(method_definition) => match method_definition.kind { - MethodDefinitionKind::Constructor => tokens.push(Cow::Borrowed("constructor")), - MethodDefinitionKind::Get => tokens.push(Cow::Borrowed("getter")), - MethodDefinitionKind::Set => tokens.push(Cow::Borrowed("setter")), - MethodDefinitionKind::Method => tokens.push(Cow::Borrowed("method")), - }, - AstKind::PropertyDefinition(_) => tokens.push(Cow::Borrowed("method")), - _ => tokens.push(Cow::Borrowed("function")), - } - - 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); - } - } else if let Some(static_name) = get_static_property_name(parent_node) { - tokens.push(static_name); - } else if let Some(name) = func.name() { - 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) = func.name() { - tokens.push(Cow::Borrowed(name.as_str())); - } - } - } - - Cow::Owned(tokens.join(" ")) -} - impl Rule for FuncNames { fn from_configuration(value: serde_json::Value) -> Self { let Some(default_value) = value.get(0) else { @@ -371,7 +250,7 @@ impl Rule for FuncNames { } fn run_once(&self, ctx: &LintContext<'_>) { - let mut invalid_funcs: Vec<(&Function, &AstNode)> = vec![]; + let mut invalid_funcs: Vec<(&Function, &AstNode, &AstNode)> = vec![]; for node in ctx.nodes() { match node.kind() { @@ -384,7 +263,7 @@ impl Rule for FuncNames { if func.generator { &self.generators_config } else { &self.default_config }; if config.is_invalid_function(func, parent_node) { - invalid_funcs.push((func, parent_node)); + invalid_funcs.push((func, node, parent_node)); } } @@ -395,7 +274,7 @@ impl Rule for FuncNames { // check at first if the callee calls an invalid function if !invalid_funcs .iter() - .filter_map(|(func, _)| func.name()) + .filter_map(|(func, _, _)| func.name()) .any(|func_name| func_name == identifier.name) { continue; @@ -418,7 +297,7 @@ impl Rule for FuncNames { // we found a recursive function, remove it from the invalid list if let Some(span) = ast_span { - invalid_funcs.retain(|(func, _)| func.span != span); + invalid_funcs.retain(|(func, _, _)| func.span != span); } } } @@ -426,8 +305,8 @@ impl Rule for FuncNames { } } - for (func, parent_node) in &invalid_funcs { - let func_name_complete = get_function_name_with_kind(func, parent_node); + for (func, node, parent_node) in invalid_funcs { + let func_name_complete = get_function_name_with_kind(node, parent_node); let report_span = Span::new(func.span.start, func.params.span.start); let replace_span = Span::new( diff --git a/crates/oxc_linter/src/rules/eslint/max_lines.rs b/crates/oxc_linter/src/rules/eslint/max_lines.rs index b2c714125fc49..fb8eb8bfc3ecd 100644 --- a/crates/oxc_linter/src/rules/eslint/max_lines.rs +++ b/crates/oxc_linter/src/rules/eslint/max_lines.rs @@ -3,7 +3,7 @@ use oxc_macros::declare_oxc_lint; use oxc_span::Span; use serde_json::Value; -use crate::{context::LintContext, rule::Rule}; +use crate::{context::LintContext, rule::Rule, utils::count_comment_lines}; fn max_lines_diagnostic(count: usize, max: usize, span: Span) -> OxcDiagnostic { OxcDiagnostic::warn(format!("File has too many lines ({count}).")) @@ -82,38 +82,11 @@ impl Rule for MaxLines { #[expect(clippy::cast_possible_truncation)] fn run_once(&self, ctx: &LintContext) { let comment_lines = if self.skip_comments { - let mut comment_lines: usize = 0; - for comment in ctx.semantic().comments() { - let comment_span = comment.content_span(); - if comment.is_line() { - let comment_line = ctx.source_text()[..comment_span.start as usize] - .lines() - .next_back() - .unwrap_or(""); - if line_has_just_comment(comment_line, "//") { - comment_lines += 1; - } - } else { - let mut start_line = - ctx.source_text()[..comment_span.start as usize].lines().count(); - let comment_start_line = ctx.source_text()[..comment_span.start as usize] - .lines() - .next_back() - .unwrap_or(""); - if !line_has_just_comment(comment_start_line, "/*") { - start_line += 1; - } - let mut end_line = - ctx.source_text()[..=comment_span.end as usize].lines().count(); - let comment_end_line = - ctx.source_text()[comment_span.end as usize..].lines().next().unwrap_or(""); - if line_has_just_comment(comment_end_line, "*/") { - end_line += 1; - } - comment_lines += end_line - start_line; - } - } - comment_lines + ctx.semantic() + .comments() + .iter() + .map(|comment| count_comment_lines(comment, ctx.source_text())) + .sum() } else { 0 }; @@ -139,14 +112,6 @@ impl Rule for MaxLines { } } -fn line_has_just_comment(line: &str, comment_chars: &str) -> bool { - if let Some(line) = line.trim().strip_prefix(comment_chars) { - line.is_empty() - } else { - false - } -} - #[test] fn test() { use crate::tester::Tester; diff --git a/crates/oxc_linter/src/rules/eslint/max_lines_per_function.rs b/crates/oxc_linter/src/rules/eslint/max_lines_per_function.rs new file mode 100644 index 0000000000000..2e71cd295ef7b --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/max_lines_per_function.rs @@ -0,0 +1,644 @@ +use std::ops::Deref; + +use oxc_ast::AstKind; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_semantic::Semantic; +use oxc_span::{GetSpan, Span}; +use serde_json::Value; + +use crate::{ + ast_util::{get_function_name_with_kind, is_function_node, iter_outer_expressions}, + context::LintContext, + rule::Rule, + utils::count_comment_lines, + AstNode, +}; + +fn max_lines_per_function_diagnostic( + name: &str, + count: usize, + max: usize, + span: Span, +) -> OxcDiagnostic { + OxcDiagnostic::warn(format!( + "The {name} has too many lines ({count}). Maximum allowed is {max}." + )) + .with_help("Consider splitting it into smaller functions.") + .with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct MaxLinesPerFunctionConfig { + max: usize, + skip_comments: bool, + skip_blank_lines: bool, + iifes: bool, +} + +#[derive(Debug, Default, Clone)] +pub struct MaxLinesPerFunction(Box); + +impl Deref for MaxLinesPerFunction { + type Target = MaxLinesPerFunctionConfig; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +declare_oxc_lint!( + /// ### What it does + /// + /// Enforce a maximum number of lines of code in a function. This rule ensures + /// that functions do not exceed a specified line count, promoting smaller, + /// more focused functions that are easier to maintain and understand. + /// + /// ### Why is this bad? + /// + /// Some people consider large functions a code smell. Large functions tend to + /// do a lot of things and can make it hard to follow what’s going on. Many coding + /// style guides dictate a limit to the number of lines that a function can + /// comprise of. This rule can help enforce that style. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule with a particular max value: + /// ```js + /// /* { "eslint/max-lines-per-function": ["error", 2] } */ + /// function foo() { + /// const x = 0; + /// } + /// + /// /* { "eslint/max-lines-per-function": ["error", 4] } */ + /// function foo() { + /// // a comment followed by a blank line + /// + /// const x = 0; + /// } + /// ``` + /// + /// Examples of **correct** code for this rule with a particular max value: + /// ```js + /// /* { "eslint/max-lines-per-function": ["error", 3] } */ + /// function foo() { + /// const x = 0; + /// } + /// + /// /* { "eslint/max-lines-per-function": ["error", 5] } */ + /// function foo() { + /// // a comment followed by a blank line + /// + /// const x = 0; + /// } + /// ``` + /// + /// ### Options + /// + /// #### max + /// + /// { type: number, default: 50 } + /// + /// The `max` enforces a maximum number of lines in a function. + /// + /// #### skipBlankLines + /// + /// { type: boolean, default: false } + /// + /// The `skipBlankLines` ignore lines made up purely of whitespace. + /// + /// #### skipComments + /// + /// { type: boolean, default: false } + /// + /// The `skipComments` ignore lines containing just comments. + /// + /// #### IIFEs + /// + /// { type: boolean, default: false } + /// + /// The `IIFEs` option controls whether IIFEs are included in the line count. + /// By default, IIFEs are not considered, but when set to `true`, they will + /// be included in the line count for the function. + /// + /// Example: + /// ```json + /// "eslint/max-lines-per-function": [ + /// "error", + /// { + /// "max": 50, + /// "skipBlankLines": false, + /// "skipComments": false, + /// "IIFEs": false + /// } + /// ] + /// ``` + MaxLinesPerFunction, + eslint, + pedantic +); + +impl Rule for MaxLinesPerFunction { + fn from_configuration(value: Value) -> Self { + let config = value.get(0); + let config = if let Some(max) = config + .and_then(Value::as_number) + .and_then(serde_json::Number::as_u64) + .and_then(|v| usize::try_from(v).ok()) + { + MaxLinesPerFunctionConfig { + max, + skip_comments: false, + skip_blank_lines: false, + iifes: false, + } + } else { + let max = config + .and_then(|config| config.get("max")) + .and_then(Value::as_number) + .and_then(serde_json::Number::as_u64) + .map_or(50, |v| usize::try_from(v).unwrap_or(50)); + let skip_comments = config + .and_then(|config| config.get("skipComments")) + .and_then(Value::as_bool) + .unwrap_or(false); + let skip_blank_lines = config + .and_then(|config| config.get("skipBlankLines")) + .and_then(Value::as_bool) + .unwrap_or(false); + let iifes = config + .and_then(|config| config.get("IIFEs")) + .and_then(Value::as_bool) + .unwrap_or(false); + + MaxLinesPerFunctionConfig { max, skip_comments, skip_blank_lines, iifes } + }; + + Self(Box::new(config)) + } + + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + if !is_function_node(node) || (!self.iifes && is_iife(node, ctx.semantic())) { + return; + } + let span = node.span(); + let source_text = ctx.source_text(); + + let comment_lines = if self.skip_comments { + ctx.semantic() + .comments_range(span.start..span.end) + .map(|comment| count_comment_lines(comment, source_text)) + .sum() + } else { + 0 + }; + + let code = &source_text[span.start as usize..span.end as usize]; + let lines_in_function = code.lines().count(); + let blank_lines = if self.skip_blank_lines { + code.lines().filter(|&line| line.trim().is_empty()).count() + } else { + 0 + }; + let result_lines = + lines_in_function.saturating_sub(blank_lines).saturating_sub(comment_lines); + if result_lines > self.max { + let name = + get_function_name_with_kind(node, ctx.nodes().parent_node(node.id()).unwrap()); + ctx.diagnostic(max_lines_per_function_diagnostic(&name, result_lines, self.max, span)); + } + } +} + +fn is_iife<'a>(node: &AstNode<'a>, semantic: &Semantic<'a>) -> bool { + let Some(AstKind::CallExpression(call)) = iter_outer_expressions(semantic, node.id()).next() + else { + return false; + }; + call.callee.span().contains_inclusive(node.span()) +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ( + "var x = 5; + var x = 2; + ", + Some(serde_json::json!([1])), + ), + ("function name() {}", Some(serde_json::json!([1]))), + ( + "function name() { + var x = 5; + var x = 2; + }", + Some(serde_json::json!([4])), + ), + ("const bar = () => 2", Some(serde_json::json!([1]))), + ( + "const bar = () => { + const x = 2 + 1; + return x; + }", + Some(serde_json::json!([4])), + ), + ( + "function name() { + var x = 5; + + + + var x = 2; + }", + Some(serde_json::json!([{ "max": 7, "skipComments": false, "skipBlankLines": false }])), + ), + ( + "function name() { + var x = 5; + + + + var x = 2; + }", + Some(serde_json::json!([{ "max": 4, "skipComments": false, "skipBlankLines": true }])), + ), + ( + "function name() { + var x = 5; + var x = 2; // end of line comment + }", + Some(serde_json::json!([{ "max": 4, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "function name() { + var x = 5; + // a comment on it's own line + var x = 2; // end of line comment + }", + Some(serde_json::json!([{ "max": 4, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "function name() { + var x = 5; + // a comment on it's own line + // and another line comment + var x = 2; // end of line comment + }", + Some(serde_json::json!([{ "max": 4, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "function name() { + var x = 5; + /* a + multi + line + comment + */ + + var x = 2; // end of line comment + }", + Some(serde_json::json!([{ "max": 5, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "function name() { + var x = 5; + /* a comment with leading whitespace */ + /* a comment with trailing whitespace */ + /* a comment with trailing and leading whitespace */ + /* a + multi + line + comment + */ + + var x = 2; // end of line comment + }", + Some(serde_json::json!([{ "max": 5, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "function foo( + aaa = 1, + bbb = 2, + ccc = 3 + ) { + return aaa + bbb + ccc + }", + Some(serde_json::json!([{ "max": 7, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "( + function + () + { + } + ) + ()", + Some( + serde_json::json!([{ "max": 4, "skipComments": true, "skipBlankLines": false, "IIFEs": true }]), + ), + ), + ( + "function parent() { + var x = 0; + function nested() { + var y = 0; + x = 2; + } + if ( x === y ) { + x++; + } + }", + Some(serde_json::json!([{ "max": 10, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "class foo { + method() { + let y = 10; + let x = 20; + return y + x; + } + }", + Some(serde_json::json!([{ "max": 5, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "(function(){ + let x = 0; + let y = 0; + let z = x + y; + let foo = {}; + return bar; + }());", + Some( + serde_json::json!([{ "max": 7, "skipComments": true, "skipBlankLines": false, "IIFEs": true }]), + ), + ), + ( + "(function(){ + let x = 0; + let y = 0; + let z = x + y; + let foo = {}; + return bar; + }());", + Some( + serde_json::json!([{ "max": 2, "skipComments": true, "skipBlankLines": false, "IIFEs": false }]), + ), + ), + ( + "(() => { + let x = 0; + let y = 0; + let z = x + y; + let foo = {}; + return bar; + })();", + Some( + serde_json::json!([{ "max": 7, "skipComments": true, "skipBlankLines": false, "IIFEs": true }]), + ), + ), + ( + "(() => { + let x = 0; + let y = 0; + let z = x + y; + let foo = {}; + return bar; + })();", + Some( + serde_json::json!([{ "max": 2, "skipComments": true, "skipBlankLines": false, "IIFEs": false }]), + ), + ), + ]; + + let repeat_60 = format!("() => {{{}}}", &"foo\n".repeat(60)); + + let fail = vec![ + ( + "function name() { + }", + Some(serde_json::json!([1])), + ), + ( + "var func = function() { + }", + Some(serde_json::json!([1])), + ), + ( + "const bar = () => { + const x = 2 + 1; + return x; + }", + Some(serde_json::json!([3])), + ), + ( + "const bar = () => + 2", + Some(serde_json::json!([1])), + ), + (&repeat_60, Some(serde_json::json!([{}]))), + ( + "function name() { + var x = 5; + + + + var x = 2; + }", + Some(serde_json::json!([{ "max": 6, "skipComments": false, "skipBlankLines": false }])), + ), + ( + "function name() { + var x = 5; + + + + var x = 2; + }", + Some(serde_json::json!([{ "max": 6, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "function name() { + var x = 5; + + + + var x = 2; + }", + Some(serde_json::json!([{ "max": 2, "skipComments": true, "skipBlankLines": true }])), + ), + ( + "function name() { + var x = 5; + + + + var x = 2; + }", + Some(serde_json::json!([{ "max": 2, "skipComments": true, "skipBlankLines": true }])), + ), + ( + "function name() { // end of line comment + var x = 5; /* mid line comment */ + // single line comment taking up whole line + + + + var x = 2; + }", + Some(serde_json::json!([{ "max": 6, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "function name() { // end of line comment + var x = 5; /* mid line comment */ + // single line comment taking up whole line + + + + var x = 2; + }", + Some(serde_json::json!([{ "max": 1, "skipComments": true, "skipBlankLines": true }])), + ), + ( + "function name() { // end of line comment + var x = 5; /* mid line comment */ + // single line comment taking up whole line + + + + var x = 2; + }", + Some(serde_json::json!([{ "max": 1, "skipComments": false, "skipBlankLines": true }])), + ), + ( + "function foo( + aaa = 1, + bbb = 2, + ccc = 3 + ) { + return aaa + bbb + ccc + }", + Some(serde_json::json!([{ "max": 2, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "( + function + () + { + } + ) + ()", + Some( + serde_json::json!([{ "max": 2, "skipComments": true, "skipBlankLines": false, "IIFEs": true }]), + ), + ), + ( + "function parent() { + var x = 0; + function nested() { + var y = 0; + x = 2; + } + if ( x === y ) { + x++; + } + }", + Some(serde_json::json!([{ "max": 9, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "function parent() { + var x = 0; + function nested() { + var y = 0; + x = 2; + } + if ( x === y ) { + x++; + } + }", + Some(serde_json::json!([{ "max": 2, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "class foo { + method() { + let y = 10; + let x = 20; + return y + x; + } + }", + Some(serde_json::json!([{ "max": 2, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "class A { + static + foo + (a) { + return a + } + }", + Some(serde_json::json!([{ "max": 2, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "var obj = { + get + foo + () { + return 1 + } + }", + Some(serde_json::json!([{ "max": 2, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "var obj = { + set + foo + ( val ) { + this._foo = val; + } + }", + Some(serde_json::json!([{ "max": 2, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "class A { + static + [ + foo + + bar + ] + (a) { + return a + } + }", + Some(serde_json::json!([{ "max": 2, "skipComments": true, "skipBlankLines": false }])), + ), + ( + "(function(){ + let x = 0; + let y = 0; + let z = x + y; + let foo = {}; + return bar; + }());", + Some( + serde_json::json!([{ "max": 2, "skipComments": true, "skipBlankLines": false, "IIFEs": true }]), + ), + ), + ( + "(() => { + let x = 0; + let y = 0; + let z = x + y; + let foo = {}; + return bar; + })();", + Some( + serde_json::json!([{ "max": 2, "skipComments": true, "skipBlankLines": false, "IIFEs": true }]), + ), + ), + ]; + + Tester::new(MaxLinesPerFunction::NAME, MaxLinesPerFunction::PLUGIN, pass, fail) + .test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/eslint_max_lines_per_function.snap b/crates/oxc_linter/src/snapshots/eslint_max_lines_per_function.snap new file mode 100644 index 0000000000000..fcbc616e7c0e1 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/eslint_max_lines_per_function.snap @@ -0,0 +1,325 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint(max-lines-per-function): The function name has too many lines (2). Maximum allowed is 1. + ╭─[max_lines_per_function.tsx:1:1] + 1 │ ╭─▶ function name() { + 2 │ ╰─▶ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function has too many lines (2). Maximum allowed is 1. + ╭─[max_lines_per_function.tsx:1:12] + 1 │ ╭─▶ var func = function() { + 2 │ ╰─▶ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function has too many lines (4). Maximum allowed is 3. + ╭─[max_lines_per_function.tsx:1:13] + 1 │ ╭─▶ const bar = () => { + 2 │ │ const x = 2 + 1; + 3 │ │ return x; + 4 │ ╰─▶ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function has too many lines (2). Maximum allowed is 1. + ╭─[max_lines_per_function.tsx:1:13] + 1 │ ╭─▶ const bar = () => + 2 │ ╰─▶ 2 + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function has too many lines (61). Maximum allowed is 50. + ╭─[max_lines_per_function.tsx:1:1] + 1 │ ╭─▶ () => {foo + 2 │ │ foo + 3 │ │ foo + 4 │ │ foo + 5 │ │ foo + 6 │ │ foo + 7 │ │ foo + 8 │ │ foo + 9 │ │ foo + 10 │ │ foo + 11 │ │ foo + 12 │ │ foo + 13 │ │ foo + 14 │ │ foo + 15 │ │ foo + 16 │ │ foo + 17 │ │ foo + 18 │ │ foo + 19 │ │ foo + 20 │ │ foo + 21 │ │ foo + 22 │ │ foo + 23 │ │ foo + 24 │ │ foo + 25 │ │ foo + 26 │ │ foo + 27 │ │ foo + 28 │ │ foo + 29 │ │ foo + 30 │ │ foo + 31 │ │ foo + 32 │ │ foo + 33 │ │ foo + 34 │ │ foo + 35 │ │ foo + 36 │ │ foo + 37 │ │ foo + 38 │ │ foo + 39 │ │ foo + 40 │ │ foo + 41 │ │ foo + 42 │ │ foo + 43 │ │ foo + 44 │ │ foo + 45 │ │ foo + 46 │ │ foo + 47 │ │ foo + 48 │ │ foo + 49 │ │ foo + 50 │ │ foo + 51 │ │ foo + 52 │ │ foo + 53 │ │ foo + 54 │ │ foo + 55 │ │ foo + 56 │ │ foo + 57 │ │ foo + 58 │ │ foo + 59 │ │ foo + 60 │ │ foo + 61 │ ╰─▶ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function name has too many lines (7). Maximum allowed is 6. + ╭─[max_lines_per_function.tsx:1:1] + 1 │ ╭─▶ function name() { + 2 │ │ var x = 5; + 3 │ │ + 4 │ │ + 5 │ │ + 6 │ │ var x = 2; + 7 │ ╰─▶ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function name has too many lines (7). Maximum allowed is 6. + ╭─[max_lines_per_function.tsx:1:1] + 1 │ ╭─▶ function name() { + 2 │ │ var x = 5; + 3 │ │ + 4 │ │ + 5 │ │ + 6 │ │ var x = 2; + 7 │ ╰─▶ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function name has too many lines (4). Maximum allowed is 2. + ╭─[max_lines_per_function.tsx:1:1] + 1 │ ╭─▶ function name() { + 2 │ │ var x = 5; + 3 │ │ + 4 │ │ + 5 │ │ + 6 │ │ var x = 2; + 7 │ ╰─▶ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function name has too many lines (4). Maximum allowed is 2. + ╭─[max_lines_per_function.tsx:1:1] + 1 │ ╭─▶ function name() { + 2 │ │ var x = 5; + 3 │ │ + 4 │ │ + 5 │ │ + 6 │ │ var x = 2; + 7 │ ╰─▶ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function name has too many lines (7). Maximum allowed is 6. + ╭─[max_lines_per_function.tsx:1:1] + 1 │ ╭─▶ function name() { // end of line comment + 2 │ │ var x = 5; /* mid line comment */ + 3 │ │ // single line comment taking up whole line + 4 │ │ + 5 │ │ + 6 │ │ + 7 │ │ var x = 2; + 8 │ ╰─▶ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function name has too many lines (4). Maximum allowed is 1. + ╭─[max_lines_per_function.tsx:1:1] + 1 │ ╭─▶ function name() { // end of line comment + 2 │ │ var x = 5; /* mid line comment */ + 3 │ │ // single line comment taking up whole line + 4 │ │ + 5 │ │ + 6 │ │ + 7 │ │ var x = 2; + 8 │ ╰─▶ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function name has too many lines (5). Maximum allowed is 1. + ╭─[max_lines_per_function.tsx:1:1] + 1 │ ╭─▶ function name() { // end of line comment + 2 │ │ var x = 5; /* mid line comment */ + 3 │ │ // single line comment taking up whole line + 4 │ │ + 5 │ │ + 6 │ │ + 7 │ │ var x = 2; + 8 │ ╰─▶ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function foo has too many lines (7). Maximum allowed is 2. + ╭─[max_lines_per_function.tsx:1:1] + 1 │ ╭─▶ function foo( + 2 │ │ aaa = 1, + 3 │ │ bbb = 2, + 4 │ │ ccc = 3 + 5 │ │ ) { + 6 │ │ return aaa + bbb + ccc + 7 │ ╰─▶ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function has too many lines (4). Maximum allowed is 2. + ╭─[max_lines_per_function.tsx:2:4] + 1 │ ( + 2 │ ╭─▶ function + 3 │ │ () + 4 │ │ { + 5 │ ╰─▶ } + 6 │ ) + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function parent has too many lines (10). Maximum allowed is 9. + ╭─[max_lines_per_function.tsx:1:1] + 1 │ ╭─▶ function parent() { + 2 │ │ var x = 0; + 3 │ │ function nested() { + 4 │ │ var y = 0; + 5 │ │ x = 2; + 6 │ │ } + 7 │ │ if ( x === y ) { + 8 │ │ x++; + 9 │ │ } + 10 │ ╰─▶ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function parent has too many lines (10). Maximum allowed is 2. + ╭─[max_lines_per_function.tsx:1:1] + 1 │ ╭─▶ function parent() { + 2 │ │ var x = 0; + 3 │ │ function nested() { + 4 │ │ var y = 0; + 5 │ │ x = 2; + 6 │ │ } + 7 │ │ if ( x === y ) { + 8 │ │ x++; + 9 │ │ } + 10 │ ╰─▶ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function nested has too many lines (4). Maximum allowed is 2. + ╭─[max_lines_per_function.tsx:3:4] + 2 │ var x = 0; + 3 │ ╭─▶ function nested() { + 4 │ │ var y = 0; + 5 │ │ x = 2; + 6 │ ╰─▶ } + 7 │ if ( x === y ) { + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The method method has too many lines (5). Maximum allowed is 2. + ╭─[max_lines_per_function.tsx:2:14] + 1 │ class foo { + 2 │ ╭─▶ method() { + 3 │ │ let y = 10; + 4 │ │ let x = 20; + 5 │ │ return y + x; + 6 │ ╰─▶ } + 7 │ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The static method foo has too many lines (3). Maximum allowed is 2. + ╭─[max_lines_per_function.tsx:4:8] + 3 │ foo + 4 │ ╭─▶ (a) { + 5 │ │ return a + 6 │ ╰─▶ } + 7 │ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function foo has too many lines (3). Maximum allowed is 2. + ╭─[max_lines_per_function.tsx:4:8] + 3 │ foo + 4 │ ╭─▶ () { + 5 │ │ return 1 + 6 │ ╰─▶ } + 7 │ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function foo has too many lines (3). Maximum allowed is 2. + ╭─[max_lines_per_function.tsx:4:8] + 3 │ foo + 4 │ ╭─▶ ( val ) { + 5 │ │ this._foo = val; + 6 │ ╰─▶ } + 7 │ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The static method has too many lines (3). Maximum allowed is 2. + ╭─[max_lines_per_function.tsx:7:8] + 6 │ ] + 7 │ ╭─▶ (a) { + 8 │ │ return a + 9 │ ╰─▶ } + 10 │ } + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function has too many lines (7). Maximum allowed is 2. + ╭─[max_lines_per_function.tsx:1:2] + 1 │ ╭─▶ (function(){ + 2 │ │ let x = 0; + 3 │ │ let y = 0; + 4 │ │ let z = x + y; + 5 │ │ let foo = {}; + 6 │ │ return bar; + 7 │ ╰─▶ }()); + ╰──── + help: Consider splitting it into smaller functions. + + ⚠ eslint(max-lines-per-function): The function has too many lines (7). Maximum allowed is 2. + ╭─[max_lines_per_function.tsx:1:2] + 1 │ ╭─▶ (() => { + 2 │ │ let x = 0; + 3 │ │ let y = 0; + 4 │ │ let z = x + y; + 5 │ │ let foo = {}; + 6 │ │ return bar; + 7 │ ╰─▶ })(); + ╰──── + help: Consider splitting it into smaller functions. diff --git a/crates/oxc_linter/src/utils/comment.rs b/crates/oxc_linter/src/utils/comment.rs new file mode 100644 index 0000000000000..4269ca4fa8095 --- /dev/null +++ b/crates/oxc_linter/src/utils/comment.rs @@ -0,0 +1,32 @@ +use oxc_ast::Comment; + +fn line_has_just_comment(line: &str, comment_chars: &str) -> bool { + if let Some(line) = line.trim().strip_prefix(comment_chars) { + line.is_empty() + } else { + false + } +} + +pub fn count_comment_lines(comment: &Comment, source_text: &str) -> usize { + let comment_span = comment.content_span(); + if comment.is_line() { + let comment_line = + source_text[..comment_span.start as usize].lines().next_back().unwrap_or(""); + usize::from(line_has_just_comment(comment_line, "//")) + } else { + let mut start_line = source_text[..comment_span.start as usize].lines().count(); + let comment_start_line = + source_text[..comment_span.start as usize].lines().next_back().unwrap_or(""); + if !line_has_just_comment(comment_start_line, "/*") { + start_line += 1; + } + let mut end_line = source_text[..=comment_span.end as usize].lines().count(); + let comment_end_line = + source_text[comment_span.end as usize..].lines().next().unwrap_or(""); + if line_has_just_comment(comment_end_line, "*/") { + end_line += 1; + } + end_line - start_line + } +} diff --git a/crates/oxc_linter/src/utils/mod.rs b/crates/oxc_linter/src/utils/mod.rs index 5faefad2fa6a6..9dec04f42b117 100644 --- a/crates/oxc_linter/src/utils/mod.rs +++ b/crates/oxc_linter/src/utils/mod.rs @@ -1,3 +1,4 @@ +mod comment; mod config; mod express; mod jest; @@ -13,8 +14,8 @@ mod vitest; use std::{io, path::Path}; pub use self::{ - config::*, express::*, jest::*, jsdoc::*, nextjs::*, promise::*, react::*, react_perf::*, - unicorn::*, url::*, vitest::*, + comment::*, config::*, express::*, jest::*, jsdoc::*, nextjs::*, promise::*, react::*, + react_perf::*, unicorn::*, url::*, vitest::*, }; /// List of Jest rules that have Vitest equivalents.