rome · ematipico · Dec 23, 2022 · Dec 19, 2022
diff --git a/crates/rome_diagnostics_categories/src/categories.rs b/crates/rome_diagnostics_categories/src/categories.rs
@@ -58,6 +58,7 @@ define_dategories! {
     "lint/nursery/noEmptyInterface": "https://docs.rome.tools/lint/rules/noEmptyInterface",
     "lint/nursery/noExtraNonNullAssertion":"https://docs.rome.tools/lint/rules/noExtraNonNullAssertion",
     "lint/nursery/noHeaderScope": "https://docs.rome.tools/lint/rules/noHeaderScope",
+    "lint/nursery/noInnerDeclarations": "https://docs.rome.tools/lint/rules/noInnerDeclarations",
     "lint/nursery/noInvalidConstructorSuper": "https://docs.rome.tools/lint/rules/noInvalidConstructorSuper",
     "lint/nursery/noNonNullAssertion": "https://docs.rome.tools/lint/rules/noNonNullAssertion",
     "lint/nursery/noPrecisionLoss": "https://docs.rome.tools/lint/rules/noPrecisionLoss",

diff --git a/crates/rome_js_analyze/src/analyzers/nursery.rs b/crates/rome_js_analyze/src/analyzers/nursery.rs
diff --git a/crates/rome_js_analyze/src/analyzers/nursery/no_inner_declarations.rs b/crates/rome_js_analyze/src/analyzers/nursery/no_inner_declarations.rs
@@ -0,0 +1,154 @@
+use rome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic};
+use rome_console::markup;
+use rome_js_syntax::{
+    AnyJsDeclaration, JsExport, JsFunctionBody, JsModuleItemList, JsScript, JsStatementList,
+    JsStaticInitializationBlockClassMember,
+};
+use rome_rowan::AstNode;
+
+use crate::control_flow::AnyJsControlFlowRoot;
+
+declare_rule! {
+    /// Disallow `function` and `var` declarations in nested blocks.
+    ///
+    /// A `function` and a `var` are accessible in the whole body of the
+    /// nearest root (function, module, script, static block).
+    /// To avoid confusion, they should be declared to the nearest root.
+    /// Note that `const` and `let` declarations are block-scoped, and therefore
+    /// they are not affected by this rule.
+    ///
+    /// Moreover, prior to ES2015 a function declaration is only allowed in
+    /// the nearest root, though parsers sometimes erroneously accept them elsewhere.
+    /// This only applies to function declarations; named or anonymous function
+    /// expressions can occur anywhere an expression is permitted.
+    ///
+    /// Source: https://eslint.org/docs/rules/no-inner-declarations
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```js,expect_diagnostic
+    /// if (test) {
+    ///     function f() {}
+    /// }
+    /// ```
+    ///
+    /// ```js,expect_diagnostic
+    /// if (test) {
+    ///     var x = 1;
+    /// }
+    /// ```
+    ///
+    /// ```js,expect_diagnostic
+    /// function f() {
+    ///     if (test) {
+    ///         function g() {}
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// ```js,expect_diagnostic
+    /// function f() {
+    ///     if (test) {
+    ///         var x = 1;
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// ### Valid
+    ///
+    /// ```js
+    /// function f() { }
+    /// ```
+    ///
+    /// ```js
+    /// function f() {
+    ///     function g() {}
+    /// }
+    /// ```
+    ///
+    /// ```js
+    /// function f() {
+    ///     var x = 1;
+    /// }
+    /// ```
+    ///
+    /// ```js
+    /// function f() {
+    ///     if (test) {
+    ///         const g = function() {};
+    ///     }
+    /// }
+    /// ```
+    ///
+    pub(crate) NoInnerDeclarations {
+        version: "next",
+        name: "noInnerDeclarations",
+        recommended: true,
+    }
+}
+
+impl Rule for NoInnerDeclarations {
+    type Query = Ast<AnyJsDeclaration>;
+    type State = ();
+    type Signals = Option<Self::State>;
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+        let decl = ctx.query();
+        let parent = match decl {
+            AnyJsDeclaration::JsFunctionDeclaration(x) => x.syntax().parent(),
+            AnyJsDeclaration::JsVariableDeclaration(x) => {
+                if x.is_var() {
+                    // ignore parent (JsVariableStatement or JsVariableDeclarationClause)
+                    x.syntax().parent()?.parent()
+                } else {
+                    None
+                }
+            }
+            _ => None,
+        }?;
+        if JsExport::can_cast(parent.kind()) || JsModuleItemList::can_cast(parent.kind()) {
+            return None;
+        }
+        if let Some(stmt_list) = JsStatementList::cast(parent) {
+            let parent_kind = stmt_list.syntax().parent()?.kind();
+            if JsFunctionBody::can_cast(parent_kind)
+                || JsScript::can_cast(parent_kind)
+                || JsStaticInitializationBlockClassMember::can_cast(parent_kind)
+            {
+                return None;
+            }
+        }
+        Some(())
+    }
+
+    fn diagnostic(ctx: &RuleContext<Self>, _: &Self::State) -> Option<RuleDiagnostic> {
+        let decl = ctx.query();
+        let decl_type = match decl {
+            AnyJsDeclaration::JsFunctionDeclaration(_) => "function",
+            _ => "var",
+        };
+        let nearest_root = decl
+            .syntax()
+            .ancestors()
+            .skip(1)
+            .find_map(AnyJsControlFlowRoot::cast)?;
+        let nearest_root_type = match nearest_root {
+            AnyJsControlFlowRoot::JsModule(_) => "module",
+            AnyJsControlFlowRoot::JsScript(_) => "script",
+            AnyJsControlFlowRoot::JsStaticInitializationBlockClassMember(_) => "static block",
+            _ => "enclosing function",
+        };
+        Some(RuleDiagnostic::new(
+            rule_category!(),
+            decl.range(),
+            markup! {
+                "This "<Emphasis>{decl_type}</Emphasis>" should be declared at the root of the "<Emphasis>{nearest_root_type}</Emphasis>"."
+            },
+        ).note(markup! {
+            "The "<Emphasis>{decl_type}</Emphasis>" is accessible in the whole body of the "<Emphasis>{nearest_root_type}</Emphasis>".\nTo avoid confusion, it should be declared at the root of the "<Emphasis>{nearest_root_type}</Emphasis>"."
+        }))
+    }
+}
diff --git a/crates/rome_js_analyze/src/control_flow/visitor.rs b/crates/rome_js_analyze/src/control_flow/visitor.rs
@@ -4,7 +4,7 @@ use rome_analyze::{merge_node_visitors, Visitor, VisitorContext};
 use rome_js_syntax::{
     AnyJsFunction, JsConstructorClassMember, JsGetterClassMember, JsGetterObjectMember, JsLanguage,
     JsMethodClassMember, JsMethodObjectMember, JsModule, JsScript, JsSetterClassMember,
-    JsSetterObjectMember,
+    JsSetterObjectMember, JsStaticInitializationBlockClassMember,
 };
 use rome_rowan::{declare_node_union, AstNode, SyntaxError, SyntaxResult};
 
@@ -168,6 +168,7 @@ declare_node_union! {
         | JsMethodClassMember
         | JsGetterClassMember
         | JsSetterClassMember
+        | JsStaticInitializationBlockClassMember
 }
 
 impl rome_analyze::NodeVisitor<ControlFlowVisitor> for FunctionVisitor {

diff --git a/crates/rome_js_analyze/tests/specs/nursery/noInnerDeclarations/invalid-module.js b/crates/rome_js_analyze/tests/specs/nursery/noInnerDeclarations/invalid-module.js
@@ -0,0 +1,5 @@
+if (foo) {
+	var a;
+	function foo() {}
+}
+export {};
diff --git a/crates/rome_js_analyze/tests/specs/nursery/noInnerDeclarations/invalid-module.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noInnerDeclarations/invalid-module.js.snap
@@ -0,0 +1,52 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+assertion_line: 92
+expression: invalid-module.js
+---
+# Input
+```js
+if (foo) {
+	var a;
+	function foo() {}
+}
+export {};
+
+```
+
+# Diagnostics
+```
+invalid-module.js:2:2 lint/nursery/noInnerDeclarations ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! This var should be declared at the root of the module.
+
+    1 │ if (foo) {
+  > 2 │ 	var a;
+      │ 	^^^^^
+    3 │ 	function foo() {}
+    4 │ }
+
+  i The var is accessible in the whole body of the module.
+    To avoid confusion, it should be declared at the root of the module.
+
+
+```
+
+```
+invalid-module.js:3:2 lint/nursery/noInnerDeclarations ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! This function should be declared at the root of the module.
+
+    1 │ if (foo) {
+    2 │ 	var a;
+  > 3 │ 	function foo() {}
+      │ 	^^^^^^^^^^^^^^^^^
+    4 │ }
+    5 │ export {};
+
+  i The function is accessible in the whole body of the module.
+    To avoid confusion, it should be declared at the root of the module.
+
+
+```
+
+
diff --git a/crates/rome_js_analyze/tests/specs/nursery/noInnerDeclarations/invalid.jsonc b/crates/rome_js_analyze/tests/specs/nursery/noInnerDeclarations/invalid.jsonc
@@ -0,0 +1,29 @@
+[
+	"if (test) { var foo; }",
+	"function doSomething() { while (test) { var foo; } }",
+
+	"if (test) { function doSomething() { } }",
+	"if (foo) var a; ",
+	"if (foo) /* some comments */ var a;",
+	"if (foo){ function f(){ if(bar){ var a; } } }",
+	"if (foo) function f(){ if(bar) var a; }",
+	"if (foo) { var fn = function(){} }",
+	"if (foo) function f(){}",
+
+	"function bar() { if (foo) function f(){}; }",
+	"function bar() { if (foo) var a; }",
+	"if (foo){ var a; }",
+	"function doSomething() { do { function somethingElse() { } } while (test); }",
+	"(function() { if (test) { function doSomething() { } } }());",
+	"while (test) { var foo; }",
+	"function doSomething() { if (test) { var foo = 42; } }",
+	"(function() { if (test) { var foo; } }());",
+	"const doSomething = () => { if (test) { var foo = 42; } }",
+	"class C { constructor() { if(test) { var foo; } } }",
+	"class C { get x() { if(test) { var foo; } } }",
+	"class C { set x() { if(test) { var foo; } } }",
+	"class C { method() { if(test) { var foo; } } }",
+	"class C { static { if (test) { function foo() {} } } }",
+	"class C { static { if (test) { var foo; } } }",
+	"class C { static { if (test) { if (anotherTest) { var foo; } } } }"
+]