diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index f2c13eeaf..4d6e5be9b 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -38,6 +38,7 @@ oxc_macros::declare_all_lint_rules! { eslint::no_shadow_restricted_names, eslint::no_unsafe_negation, eslint::no_unused_labels, + eslint::require_yield, eslint::use_isnan, eslint::valid_typeof, typescript::isolated_declaration diff --git a/crates/oxc_linter/src/rules/eslint/require_yield.rs b/crates/oxc_linter/src/rules/eslint/require_yield.rs new file mode 100644 index 000000000..aebd71bb3 --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/require_yield.rs @@ -0,0 +1,76 @@ +use oxc_ast::AstKind; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::{GetSpan, Span}; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint(require-yield): This generator function does not have 'yield'")] +#[diagnostic(severity(warning))] +struct RequireYieldDiagnostic(#[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct RequireYield; + +declare_oxc_lint!( + /// ### What it does + /// + /// This rule generates warnings for generator functions that do not have the yield keyword. + /// + /// ### Why is this bad? + /// + /// Probably a mistake. + /// + /// ### Example + /// ```javascript + /// function* foo() { + /// return 10; + /// } + /// ``` + RequireYield, + correctness +); + +impl Rule for RequireYield { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let kind = node.kind(); + if (matches!(kind, AstKind::Function(func) if func.generator && func.body.as_ref().is_some_and(|body| !body.statements.is_empty())) + || matches!(kind, AstKind::ArrowExpression(arrow) if arrow.generator && !arrow.body.statements.is_empty())) + && !node.flags().has_yield() + { + ctx.diagnostic(RequireYieldDiagnostic(kind.span())); + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("function foo() { return 0; }", None), + ("function* foo() { yield 0; }", None), + ("function* foo() { }", None), + ("(function* foo() { yield 0; })();", None), + ("(function* foo() { })();", None), + ("var obj = { *foo() { yield 0; } };", None), + ("var obj = { *foo() { } };", None), + ("class A { *foo() { yield 0; } };", None), + ("class A { *foo() { } };", None), + ]; + + let fail = vec![ + ("function* foo() { return 0; }", None), + ("(function* foo() { return 0; })();", None), + ("var obj = { *foo() { return 0; } }", None), + ("class A { *foo() { return 0; } }", None), + ("function* foo() { function* bar() { yield 0; } }", None), + ("function* foo() { function* bar() { return 0; } yield 0; }", None), + ]; + + Tester::new(RequireYield::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/require_yield.snap b/crates/oxc_linter/src/snapshots/require_yield.snap new file mode 100644 index 000000000..f99c4cf24 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/require_yield.snap @@ -0,0 +1,41 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: require_yield +--- + ⚠ eslint(require-yield): This generator function does not have 'yield' + ╭─[require_yield.tsx:1:1] + 1 │ function* foo() { return 0; } + · ───────────────────────────── + ╰──── + + ⚠ eslint(require-yield): This generator function does not have 'yield' + ╭─[require_yield.tsx:1:1] + 1 │ (function* foo() { return 0; })(); + · ───────────────────────────── + ╰──── + + ⚠ eslint(require-yield): This generator function does not have 'yield' + ╭─[require_yield.tsx:1:1] + 1 │ var obj = { *foo() { return 0; } } + · ──────────────── + ╰──── + + ⚠ eslint(require-yield): This generator function does not have 'yield' + ╭─[require_yield.tsx:1:1] + 1 │ class A { *foo() { return 0; } } + · ──────────────── + ╰──── + + ⚠ eslint(require-yield): This generator function does not have 'yield' + ╭─[require_yield.tsx:1:1] + 1 │ function* foo() { function* bar() { yield 0; } } + · ──────────────────────────────────────────────── + ╰──── + + ⚠ eslint(require-yield): This generator function does not have 'yield' + ╭─[require_yield.tsx:1:1] + 1 │ function* foo() { function* bar() { return 0; } yield 0; } + · ───────────────────────────── + ╰──── + + diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 718b6d646..80ff8b460 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -48,6 +48,8 @@ pub struct SemanticBuilder<'a> { pub current_node_flags: NodeFlags, pub current_symbol_flags: SymbolFlags, pub current_scope_id: ScopeId, + /// Stores current `AstKind::Function` and `AstKind::ArrowExpression` during AST visit + pub function_stack: Vec, // builders pub nodes: AstNodes<'a>, @@ -83,6 +85,7 @@ impl<'a> SemanticBuilder<'a> { current_node_flags: NodeFlags::empty(), current_symbol_flags: SymbolFlags::empty(), current_scope_id, + function_stack: vec![], nodes: AstNodes::default(), scope, symbols: SymbolTable::default(), @@ -214,6 +217,12 @@ impl<'a> SemanticBuilder<'a> { || self.current_node_flags.contains(NodeFlags::Class) } + pub fn set_function_node_flag(&mut self, flag: NodeFlags) { + if let Some(current_function) = self.function_stack.last() { + *self.nodes.get_node_mut(*current_function).flags_mut() |= flag; + } + } + /// Declares a `Symbol` for the node, adds it to symbol table, and binds it to the scope. /// /// includes: the `SymbolFlags` that node has in addition to its declaration type (eg: export, ambient, etc.) @@ -431,8 +440,12 @@ impl<'a> SemanticBuilder<'a> { decl.bind(self); } AstKind::Function(func) => { + self.function_stack.push(self.current_node_id); func.bind(self); } + AstKind::ArrowExpression(_) => { + self.function_stack.push(self.current_node_id); + } AstKind::Class(class) => { self.current_node_flags |= NodeFlags::Class; class.bind(self); @@ -475,6 +488,9 @@ impl<'a> SemanticBuilder<'a> { } } } + AstKind::YieldExpression(_) => { + self.set_function_node_flag(NodeFlags::HasYield); + } _ => {} } } @@ -495,6 +511,9 @@ impl<'a> SemanticBuilder<'a> { } self.unused_labels.curr_scope = scope.parent; } + AstKind::Function(_) | AstKind::ArrowExpression(_) => { + self.function_stack.pop(); + } _ => {} } } diff --git a/crates/oxc_semantic/src/jsdoc/mod.rs b/crates/oxc_semantic/src/jsdoc/mod.rs index 6292b7ca5..561f98a47 100644 --- a/crates/oxc_semantic/src/jsdoc/mod.rs +++ b/crates/oxc_semantic/src/jsdoc/mod.rs @@ -30,7 +30,7 @@ impl<'a> JSDoc<'a> { } pub fn get_by_node<'b>(&'b self, node: &AstNode<'a>) -> Option> { - if !node.has_jsdoc() { + if !node.flags().has_jsdoc() { return None; } let span = node.kind().span(); diff --git a/crates/oxc_semantic/src/lib.rs b/crates/oxc_semantic/src/lib.rs index d6e4a5f66..7db002eba 100644 --- a/crates/oxc_semantic/src/lib.rs +++ b/crates/oxc_semantic/src/lib.rs @@ -23,7 +23,7 @@ pub use oxc_syntax::{ }; pub use crate::{ - node::{AstNode, AstNodeId, AstNodes}, + node::{AstNode, AstNodeId, AstNodes, NodeFlags}, reference::{Reference, ReferenceFlag, ReferenceId}, scope::ScopeTree, symbol::SymbolTable, diff --git a/crates/oxc_semantic/src/node.rs b/crates/oxc_semantic/src/node.rs index 904bb18cd..c553e8cd8 100644 --- a/crates/oxc_semantic/src/node.rs +++ b/crates/oxc_semantic/src/node.rs @@ -13,8 +13,24 @@ define_index_type! { bitflags! { #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct NodeFlags: u8 { - const JSDoc = 1 << 0; // If the Node has a JSDoc comment attached - const Class = 1 << 1; // If Node is inside a class + const JSDoc = 1 << 0; // If the Node has a JSDoc comment attached + const Class = 1 << 1; // If Node is inside a class + const HasYield = 1 << 2; // If function has yield statement + + } +} + +impl NodeFlags { + pub fn has_jsdoc(&self) -> bool { + self.contains(Self::JSDoc) + } + + pub fn has_class(&self) -> bool { + self.contains(Self::Class) + } + + pub fn has_yield(&self) -> bool { + self.contains(Self::HasYield) } } @@ -48,17 +64,17 @@ impl<'a> AstNode<'a> { self.scope_id } + pub fn flags(&self) -> NodeFlags { + self.flags + } + + pub fn flags_mut(&mut self) -> &mut NodeFlags { + &mut self.flags + } + pub fn strict_mode(&self, flags: ScopeFlags) -> bool { // All parts of a ClassDeclaration or a ClassExpression are strict mode code. - flags.is_strict_mode() || self.in_class() - } - - pub fn in_class(self) -> bool { - self.flags.contains(NodeFlags::Class) - } - - pub fn has_jsdoc(&self) -> bool { - self.flags.contains(NodeFlags::JSDoc) + flags.is_strict_mode() || self.flags.has_class() } } @@ -94,6 +110,10 @@ impl<'a> AstNodes<'a> { &self.nodes[ast_node_id] } + pub fn get_node_mut(&mut self, ast_node_id: AstNodeId) -> &mut AstNode<'a> { + &mut self.nodes[ast_node_id] + } + pub fn ancestors(&self, ast_node_id: AstNodeId) -> impl Iterator + '_ { let parent_ids = &self.parent_ids; std::iter::successors(Some(ast_node_id), |node_id| parent_ids[*node_id])