diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 053fec48b501e3..cf0f93ced50a48 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -152,6 +152,7 @@ mod eslint { pub mod unicode_bom; pub mod use_isnan; pub mod valid_typeof; + pub mod vars_on_top; pub mod yoda; } @@ -644,6 +645,7 @@ oxc_macros::declare_all_lint_rules! { eslint::unicode_bom, eslint::use_isnan, eslint::valid_typeof, + eslint::vars_on_top, eslint::yoda, import::default, import::export, diff --git a/crates/oxc_linter/src/rules/eslint/vars_on_top.rs b/crates/oxc_linter/src/rules/eslint/vars_on_top.rs new file mode 100644 index 00000000000000..3d88b0b842e881 --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/vars_on_top.rs @@ -0,0 +1,543 @@ +use oxc_ast::ast::{Declaration, Expression, Program, Statement, VariableDeclarationKind}; +use oxc_ast::AstKind; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::{GetSpan, Span}; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +fn vars_on_top_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("All 'var' declarations must be at the top of the function scope.") + .with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct VarsOnTop; + +declare_oxc_lint!( + /// ### What it does + /// + /// Enforces that all `var` declarations are placed at the top of their containing scope. + /// + /// ### Why is this bad? + /// + /// In JavaScript, `var` declarations are hoisted to the top of their containing scope. Placing `var` declarations at the top explicitly improves code readability and maintainability by making the scope of variables clear. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// function doSomething() { + /// if (true) { + /// var first = true; + /// } + /// var second; + /// } + /// + /// function doSomethingElse() { + /// for (var i = 0; i < 10; i++) {} + /// } + /// + /// f(); + /// var a; + /// + /// class C { + /// static { + /// if (something) { + /// var a = true; + /// } + /// } + /// static { + /// f(); + /// var a; + /// } + /// } + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// function doSomething() { + /// var first; + /// var second; + /// if (true) { + /// first = true; + /// } + /// } + /// + /// function doSomethingElse() { + /// var i; + /// for (i = 0; i < 10; i++) {} + /// } + /// + /// var a; + /// f(); + /// + /// class C { + /// static { + /// var a; + /// if (something) { + /// a = true; + /// } + /// } + /// static { + /// var a; + /// f(); + /// } + /// } + /// ``` + VarsOnTop, + style, +); + +impl Rule for VarsOnTop { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::VariableDeclaration(declaration) = node.kind() else { + return; + }; + if declaration.kind != VariableDeclarationKind::Var { + return; + } + let Some(parent) = ctx.nodes().parent_node(node.id()) else { + return; + }; + + match parent.kind() { + AstKind::ExportNamedDeclaration(_) => { + if let Some(grand_parent) = ctx.nodes().parent_node(parent.id()) { + if let AstKind::Program(grand_parent) = grand_parent.kind() { + global_var_check(parent, grand_parent, ctx); + } + } + } + AstKind::Program(parent) => { + global_var_check(node, parent, ctx); + } + _ => block_scope_var_check(node, ctx), + } + } +} + +fn looks_like_directive(node: &Statement) -> bool { + matches!( + node, + Statement::ExpressionStatement(expr_stmt) if matches!( + &expr_stmt.expression, + Expression::StringLiteral(_) + ) + ) +} + +fn looks_like_import(node: &Statement) -> bool { + matches!(node, Statement::ImportDeclaration(_)) +} + +fn is_variable_declaration(node: &Statement) -> bool { + if matches!(node, Statement::VariableDeclaration(_)) { + return true; + } + + if let Statement::ExportNamedDeclaration(export) = node { + return matches!(export.declaration, Some(Declaration::VariableDeclaration(_))); + } + + false +} + +fn is_var_on_top(node: &AstNode, statements: &[Statement], ctx: &LintContext) -> bool { + let mut i = 0; + let len = statements.len(); + let parent = ctx.nodes().parent_node(node.id()); + + if let Some(parent) = parent { + if !matches!(parent.kind(), AstKind::StaticBlock(_)) { + while i < len { + if !looks_like_directive(&statements[i]) && !looks_like_import(&statements[i]) { + break; + } + i += 1; + } + } + } + + let node_span = node.span(); + while i < len { + if !is_variable_declaration(&statements[i]) { + return false; + } + let stmt_span = statements[i].span(); + + if stmt_span == node_span { + return true; + } + i += 1; + } + + false +} + +fn global_var_check(node: &AstNode, parent: &Program, ctx: &LintContext) { + if !is_var_on_top(node, &parent.body, ctx) { + ctx.diagnostic(vars_on_top_diagnostic(node.span())); + } +} + +fn block_scope_var_check(node: &AstNode, ctx: &LintContext) { + if let Some(parent) = ctx.nodes().parent_node(node.id()) { + match parent.kind() { + AstKind::BlockStatement(block) => { + if check_var_on_top_in_function_scope(node, &block.body, parent, ctx) { + return; + } + } + AstKind::FunctionBody(block) => { + if check_var_on_top_in_function_scope(node, &block.statements, parent, ctx) { + return; + } + } + AstKind::StaticBlock(block) => { + if is_var_on_top(node, &block.body, ctx) { + return; + } + } + _ => {} + } + } + ctx.diagnostic(vars_on_top_diagnostic(node.span())); +} + +fn check_var_on_top_in_function_scope( + node: &AstNode, + statements: &[Statement], + parent: &AstNode, + ctx: &LintContext, +) -> bool { + if let Some(grandparent) = ctx.nodes().parent_node(parent.id()) { + if matches!( + grandparent.kind(), + AstKind::Function(_) | AstKind::FunctionBody(_) | AstKind::ArrowFunctionExpression(_) + ) && is_var_on_top(node, statements, ctx) + { + return true; + } + } + + false +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + "var first = 0; + function foo() { + first = 2; + } + ", +"function foo() { + } + ", +"function foo() { + var first; + if (true) { + first = true; + } else { + first = 1; + } + } + ", +"function foo() { + var first; + var second = 1; + var third; + var fourth = 1, fifth, sixth = third; + var seventh; + if (true) { + third = true; + } + first = second; + } + ", +"function foo() { + var i; + for (i = 0; i < 10; i++) { + alert(i); + } + } + ", +"function foo() { + var outer; + function inner() { + var inner = 1; + var outer = inner; + } + outer = 1; + } + ", +"function foo() { + var first; + //Hello + var second = 1; + first = second; + } + ", +"function foo() { + var first; + /* + Hello Clarice + */ + var second = 1; + first = second; + } + ", +"function foo() { + var first; + var second = 1; + function bar(){ + var first; + first = 5; + } + first = second; + } + ", +"function foo() { + var first; + var second = 1; + function bar(){ + var third; + third = 5; + } + first = second; + } + ", +"function foo() { + var first; + var bar = function(){ + var third; + third = 5; + } + first = 5; + } + ", +"function foo() { + var first; + first.onclick(function(){ + var third; + third = 5; + }); + first = 5; + } + ", +"function foo() { + var i = 0; + for (let j = 0; j < 10; j++) { + alert(j); + } + i = i + 1; + }", // { "ecmaVersion": 6 }, +"'use strict'; var x; f();", +"'use strict'; 'directive'; var x; var y; f();", +"function f() { 'use strict'; var x; f(); }", +"function f() { 'use strict'; 'directive'; var x; var y; f(); }", +"import React from 'react'; var y; function f() { 'use strict'; var x; var y; f(); }", // { "ecmaVersion": 6, "sourceType": "module" }, +"'use strict'; import React from 'react'; var y; function f() { 'use strict'; var x; var y; f(); }", // { "ecmaVersion": 6, "sourceType": "module" }, +"import React from 'react'; 'use strict'; var y; function f() { 'use strict'; var x; var y; f(); }", // { "ecmaVersion": 6, "sourceType": "module" }, +"import * as foo from 'mod.js'; 'use strict'; var y; function f() { 'use strict'; var x; var y; f(); }", // { "ecmaVersion": 6, "sourceType": "module" }, +"import { square, diag } from 'lib'; 'use strict'; var y; function f() { 'use strict'; var x; var y; f(); }", // { "ecmaVersion": 6, "sourceType": "module" }, +"import { default as foo } from 'lib'; 'use strict'; var y; function f() { 'use strict'; var x; var y; f(); }", // { "ecmaVersion": 6, "sourceType": "module" }, +"import 'src/mylib'; 'use strict'; var y; function f() { 'use strict'; var x; var y; f(); }", // { "ecmaVersion": 6, "sourceType": "module" }, +"import theDefault, { named1, named2 } from 'src/mylib'; 'use strict'; var y; function f() { 'use strict'; var x; var y; f(); }", // { "ecmaVersion": 6, "sourceType": "module" }, +"export var x; + var y; + var z;", // { "ecmaVersion": 6, "sourceType": "module" }, +"var x; + export var y; + var z;", // { "ecmaVersion": 6, "sourceType": "module" }, +"var x; + var y; + export var z;", // { "ecmaVersion": 6, "sourceType": "module" }, +"class C { + static { + var x; + } + }", // { "ecmaVersion": 2022 }, +"class C { + static { + var x; + foo(); + } + }", // { "ecmaVersion": 2022 }, +"class C { + static { + var x; + var y; + } + }", // { "ecmaVersion": 2022 }, +"class C { + static { + var x; + var y; + foo(); + } + }", // { "ecmaVersion": 2022 }, +"class C { + static { + let x; + var y; + } + }", // { "ecmaVersion": 2022 }, +"class C { + static { + foo(); + let x; + } + }", // { "ecmaVersion": 2022 } + ]; + + let fail = vec![ + "var first = 0; + function foo() { + first = 2; + second = 2; + } + var second = 0;", + "function foo() { + var first; + first = 1; + first = 2; + first = 3; + first = 4; + var second = 1; + second = 2; + first = second; + }", + "function foo() { + var first; + if (true) { + var second = true; + } + first = second; + }", + "function foo() { + for (var i = 0; i < 10; i++) { + alert(i); + } + }", + "function foo() { + var first = 10; + var i; + for (i = 0; i < first; i ++) { + var second = i; + } + }", + "function foo() { + var first = 10; + var i; + switch (first) { + case 10: + var hello = 1; + break; + } + }", + "function foo() { + var first = 10; + var i; + try { + var hello = 1; + } catch (e) { + alert('error'); + } + }", + "function foo() { + var first = 10; + var i; + try { + asdf; + } catch (e) { + var hello = 1; + } + }", + "function foo() { + var first = 10; + while (first) { + var hello = 1; + } + }", + "function foo() { + var first = 10; + do { + var hello = 1; + } while (first == 10); + }", + "function foo() { + var first = [1,2,3]; + for (var item in first) { + item++; + } + }", + "function foo() { + var first = [1,2,3]; + var item; + for (item in first) { + var hello = item; + } + }", + "var foo = () => { + var first = [1,2,3]; + var item; + for (item in first) { + var hello = item; + } + }", // { "ecmaVersion": 6 }, + "'use strict'; 0; var x; f();", + "'use strict'; var x; 'directive'; var y; f();", + "function f() { 'use strict'; 0; var x; f(); }", + "function f() { 'use strict'; var x; 'directive'; var y; f(); }", + "export function f() {} + var x;", // { "ecmaVersion": 6, "sourceType": "module" }, + "var x; + export function f() {} + var y;", // { "ecmaVersion": 6, "sourceType": "module" }, + "import {foo} from 'foo'; + export {foo}; + var test = 1;", // { "ecmaVersion": 6, "sourceType": "module" }, + "export {foo} from 'foo'; + var test = 1;", // { "ecmaVersion": 6, "sourceType": "module" }, + "export * from 'foo'; + var test = 1;", // { "ecmaVersion": 6, "sourceType": "module" }, + "class C { + static { + foo(); + var x; + } + }", // { "ecmaVersion": 2022 }, + "class C { + static { + 'use strict'; + var x; + } + }", // { "ecmaVersion": 2022 }, + "class C { + static { + var x; + foo(); + var y; + } + }", // { "ecmaVersion": 2022 }, + "class C { + static { + if (foo) { + var x; + } + } + }", // { "ecmaVersion": 2022 }, + "class C { + static { + if (foo) + var x; + } + }", // { "ecmaVersion": 2022 } + ]; + + Tester::new(VarsOnTop::NAME, VarsOnTop::CATEGORY, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/eslint_vars_on_top.snap.new b/crates/oxc_linter/src/snapshots/eslint_vars_on_top.snap.new new file mode 100644 index 00000000000000..24a457b414ed6a --- /dev/null +++ b/crates/oxc_linter/src/snapshots/eslint_vars_on_top.snap.new @@ -0,0 +1,206 @@ +--- +source: crates/oxc_linter/src/tester.rs +assertion_line: 356 +snapshot_kind: text +--- + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:6:4] + 5 │ } + 6 │ var second = 0; + · ─────────────── + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:7:7] + 6 │ first = 4; + 7 │ var second = 1; + · ─────────────── + 8 │ second = 2; + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:4:11] + 3 │ if (true) { + 4 │ var second = true; + · ────────────────── + 5 │ } + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:2:12] + 1 │ function foo() { + 2 │ for (var i = 0; i < 10; i++) { + · ───────── + 3 │ alert(i); + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:5:11] + 4 │ for (i = 0; i < first; i ++) { + 5 │ var second = i; + · ─────────────── + 6 │ } + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:6:15] + 5 │ case 10: + 6 │ var hello = 1; + · ────────────── + 7 │ break; + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:5:11] + 4 │ try { + 5 │ var hello = 1; + · ────────────── + 6 │ } catch (e) { + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:7:11] + 6 │ } catch (e) { + 7 │ var hello = 1; + · ────────────── + 8 │ } + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:4:11] + 3 │ while (first) { + 4 │ var hello = 1; + · ────────────── + 5 │ } + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:4:11] + 3 │ do { + 4 │ var hello = 1; + · ────────────── + 5 │ } while (first == 10); + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:3:12] + 2 │ var first = [1,2,3]; + 3 │ for (var item in first) { + · ──────── + 4 │ item++; + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:5:11] + 4 │ for (item in first) { + 5 │ var hello = item; + · ───────────────── + 6 │ } + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:5:11] + 4 │ for (item in first) { + 5 │ var hello = item; + · ───────────────── + 6 │ } + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:1:18] + 1 │ 'use strict'; 0; var x; f(); + · ────── + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:1:35] + 1 │ 'use strict'; var x; 'directive'; var y; f(); + · ────── + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:1:33] + 1 │ function f() { 'use strict'; 0; var x; f(); } + · ────── + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:1:51] + 1 │ function f() { 'use strict'; var x; 'directive'; var y; f(); } + · ────── + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:2:4] + 1 │ export function f() {} + 2 │ var x; + · ────── + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:3:4] + 2 │ export function f() {} + 3 │ var y; + · ────── + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:3:4] + 2 │ export {foo}; + 3 │ var test = 1; + · ───────────── + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:2:4] + 1 │ export {foo} from 'foo'; + 2 │ var test = 1; + · ───────────── + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:2:4] + 1 │ export * from 'foo'; + 2 │ var test = 1; + · ───────────── + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:4:12] + 3 │ foo(); + 4 │ var x; + · ────── + 5 │ } + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:4:12] + 3 │ 'use strict'; + 4 │ var x; + · ────── + 5 │ } + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:5:12] + 4 │ foo(); + 5 │ var y; + · ────── + 6 │ } + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:4:16] + 3 │ if (foo) { + 4 │ var x; + · ────── + 5 │ } + ╰──── + + ⚠ eslint(vars-on-top): All 'var' declarations must be at the top of the function scope. + ╭─[vars_on_top.tsx:4:16] + 3 │ if (foo) + 4 │ var x; + · ────── + 5 │ } + ╰────