fix(rome_js_analyze): allow nested func declarations in esm

CHANGELOG.md

@@ -7,6 +7,11 @@
 ## Editors
 ## Formatter
 ## Linter
+
+#### Other changes
+
+- `noInnerDeclarations`: allow function declarations in nested block inside an _ES module_ [#4492](https://github.com/rome/tools/compare/main...Conaclos:noInnerDeclarations/4492?expand=1).
+
 ## Parser
 ## VSCode
 ## JavaScript APIs

crates/rome_js_analyze/src/analyzers/correctness/no_inner_declarations.rs

@@ -1,34 +1,34 @@
 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,
+    AnyJsDeclaration, JsExport, JsFileSource, JsFunctionBody, JsModuleItemList, JsScript,
+    JsStatementList, JsStaticInitializationBlockClassMember,
 };
 use rome_rowan::AstNode;
 
 use crate::control_flow::AnyJsControlFlowRoot;
 
 declare_rule! {
-    /// Disallow `function` and `var` declarations in nested blocks.
+    /// Disallow `function` and `var` declarations that are accessible outside their block.
     ///
-    /// A `function` and a `var` are accessible in the whole body of the
-    /// nearest root (function, module, script, static block).
+    /// A `var` is 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.
+    /// Prior to ES2015, `function` declarations were only allowed in the nearest root,
+    /// though parsers sometimes erroneously accept them elsewhere.
+    /// In ES2015, inside an _ES module_, a `function` declaration is always block-scoped.
+    ///
+    /// Note that `const` and `let` declarations are block-scoped,
+    /// and therefore they are not affected by this rule.
+    /// Moreover, `function` declarations in nested blocks are allowed inside _ES modules_.
     ///
     /// Source: https://eslint.org/docs/rules/no-inner-declarations
     ///
     /// ## Examples
     ///
     /// ### Invalid
     ///
-    /// ```js,expect_diagnostic
+    /// ```cjs,expect_diagnostic
     /// if (test) {
     ///     function f() {}
     /// }
@@ -40,7 +40,7 @@ declare_rule! {
     /// }
     /// ```
     ///
-    /// ```js,expect_diagnostic
+    /// ```cjs,expect_diagnostic
     /// function f() {
     ///     if (test) {
     ///         function g() {}
@@ -59,6 +59,14 @@ declare_rule! {
     /// ### Valid
     ///
     /// ```js
+    /// // inside a module, function declarations are block-scoped and thus allowed.
+    /// if (test) {
+    ///     function f() {}
+    /// }
+    /// export {}
+    /// ```
+    ///
+    /// ```js
     /// function f() { }
     /// ```
     ///
@@ -98,7 +106,14 @@ impl Rule for NoInnerDeclarations {
     fn run(ctx: &RuleContext<Self>) -> Self::Signals {
         let decl = ctx.query();
         let parent = match decl {
-            AnyJsDeclaration::JsFunctionDeclaration(x) => x.syntax().parent(),
+            AnyJsDeclaration::JsFunctionDeclaration(x) => {
+                if ctx.source_type::<JsFileSource>().is_module() {
+                    // In strict mode (implied by esm), function declarations are block-scoped.
+                    None
+                } else {
+                    x.syntax().parent()
+                }
+            }
             AnyJsDeclaration::JsVariableDeclaration(x) => {
                 if x.is_var() {
                     // ignore parent (JsVariableStatement or JsVariableDeclarationClause)

crates/rome_js_analyze/tests/specs/correctness/noInnerDeclarations/invalid-module.js → crates/rome_js_analyze/tests/specs/correctness/noInnerDeclarations/invalid-module.cjs

@@ -1,5 +1,4 @@
 if (foo) {
 	var a;
 	function foo() {}
-}
-export {};
+}

crates/rome_js_analyze/tests/specs/correctness/noInnerDeclarations/invalid-module.cjs.snap

@@ -0,0 +1,49 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+assertion_line: 96
+expression: invalid-module.cjs
+---
+# Input
+```js
+if (foo) {
+	var a;
+	function foo() {}
+}
+```
+
+# Diagnostics
+```
+invalid-module.cjs:2:2 lint/correctness/noInnerDeclarations ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! This var should be declared at the root of the script.
+  
+    1 │ if (foo) {
+  > 2 │ 	var a;
+      │ 	^^^^^
+    3 │ 	function foo() {}
+    4 │ }
+  
+  i The var is accessible in the whole body of the script.
+    To avoid confusion, it should be declared at the root of the script.
+  
+
+```
+
+```
+invalid-module.cjs:3:2 lint/correctness/noInnerDeclarations ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! This function should be declared at the root of the script.
+  
+    1 │ if (foo) {
+    2 │ 	var a;
+  > 3 │ 	function foo() {}
+      │ 	^^^^^^^^^^^^^^^^^
+    4 │ }
+  
+  i The function is accessible in the whole body of the script.
+    To avoid confusion, it should be declared at the root of the script.
+  
+
+```
+
+

crates/rome_js_analyze/tests/specs/correctness/noInnerDeclarations/valid-block-scoped-func-decl.js

@@ -0,0 +1,6 @@
+// ensure we are in esm
+export {}
+
+{
+    function f() {}
+}

crates/rome_js_analyze/tests/specs/correctness/noInnerDeclarations/valid-block-scoped-func-decl.js.snap

@@ -0,0 +1,17 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+assertion_line: 96
+expression: valid-block-scoped-func-decl.js
+---
+# Input
+```js
+// ensure we are in esm
+export {}
+
+{
+    function f() {}
+}
+
+```
+
+

website/src/pages/lint/rules/index.mdx

@@ -304,7 +304,7 @@ Disallow calling global object properties as functions
 	<a href="/lint/rules/noInnerDeclarations">noInnerDeclarations</a>
 	<span class="recommended">recommended</span>
 </h3>
-Disallow <code>function</code> and <code>var</code> declarations in nested blocks.
+Disallow <code>function</code> and <code>var</code> declarations that are accessible outside their block.
 </section>
 <section class="rule">
 <h3 data-toc-exclude id="noInvalidConstructorSuper">