feat!: no-inner-declaration new default behaviour and option (#17885)

Tanujkanti4441 · web-flow · commit 701f1afbee34 · 2024-01-09T14:09:34.000-05:00
* feat: no-inner-declaration new default behaviour and option

* feat!: removed no-inner-declarations from recommended

* docs: added legacy option

* feat!: updated the legacy option

* feat!: added more test and updated docs

* feat: no-inner-declaration new default behaviour and option

* feat!: removed no-inner-declarations from recommended

* docs: added legacy option

* feat!: updated the legacy option

* feat!: added more test and updated docs

* added test for module

* remove non-strict report for variablDeclarations

* change legacy option to blockScopedFunctions

* check upper scope

* update docs and tests

* update docs

* add functions option in docs
diff --git a/docs/src/rules/no-inner-declarations.md b/docs/src/rules/no-inner-declarations.md
@@ -30,6 +30,20 @@ function anotherThing() {
 }
 ```
 
+In ES6, [block-level functions](https://leanpub.com/understandinges6/read#leanpub-auto-block-level-functions) (functions declared inside a block) are limited to the scope of the block they are declared in and outside of the block scope they can't be accessed and called, but only when the code is in strict mode (code with `"use strict"` tag or ESM modules). In non-strict mode, they can be accessed and called outside of the block scope.
+
+```js
+"use strict";
+
+if (test) {
+    function doSomething () { }
+
+    doSomething(); // no error
+}
+
+doSomething(); // error
+```
+
 A variable declaration is permitted anywhere a statement can go, even nested deeply inside other blocks. This is often undesirable due to variable hoisting, and moving declarations to the root of the program or function body can increase clarity. Note that [block bindings](https://leanpub.com/understandinges6/read#leanpub-auto-block-bindings) (`let`, `const`) are not hoisted and therefore they are not affected by this rule.
 
 ```js
@@ -65,10 +79,11 @@ This rule requires that function declarations and, optionally, variable declarat
 
 ## Options
 
-This rule has a string option:
+This rule has a string and an object option:
 
 * `"functions"` (default) disallows `function` declarations in nested blocks
 * `"both"` disallows `function` and `var` declarations in nested blocks
+* `{ blockScopedFunctions: "allow" }` (default) this option allows `function` declarations in nested blocks when code is in strict mode (code with `"use strict"` tag or ESM modules) and `languageOptions.ecmaVersion` is set to `2015` or above. This option can be disabled by setting it to `"disallow"`.
 
 ### functions
 
@@ -79,6 +94,8 @@ Examples of **incorrect** code for this rule with the default `"functions"` opti
 ```js
 /*eslint no-inner-declarations: "error"*/
 
+// script, non-strict code
+
 if (test) {
     function doSomething() { }
 }
@@ -90,14 +107,6 @@ function doSomethingElse() {
 }
 
 if (foo) function f(){}
-
-class C {
-    static {
-        if (test) {
-            function doSomething() { }
-        }
-    }
-}
 ```
 
 :::
@@ -115,6 +124,14 @@ function doSomethingElse() {
     function doAnotherThing() { }
 }
 
+function doSomethingElse() {
+    "use strict";
+    
+    if (test) {
+        function doAnotherThing() { }
+    }
+}
+
 class C {
     static {
         function doSomething() { }
@@ -195,6 +212,128 @@ class C {
 
 :::
 
+### blockScopedFunctions
+
+Example of **incorrect** code for this rule with `{ blockScopedFunctions: "disallow" }` option with `ecmaVersion: 2015`:
+
+::: incorrect { "sourceType": "script", "ecmaVersion": 2015 }
+
+```js
+/*eslint no-inner-declarations: ["error", "functions", { blockScopedFunctions: "disallow" }]*/
+
+// non-strict code
+
+if (test) {
+    function doSomething() { }
+}
+
+function doSomething() {
+    if (test) {
+        function doSomethingElse() { }
+    }
+}
+
+// strict code
+
+function foo() {
+    "use strict";
+
+    if (test) {
+        function bar() { }
+    }
+}
+```
+
+:::
+
+Example of **correct** code for this rule with `{ blockScopedFunctions: "disallow" }` option with `ecmaVersion: 2015`:
+
+::: correct { "sourceType": "script", "ecmaVersion": 2015 }
+
+```js
+/*eslint no-inner-declarations: ["error", "functions", { blockScopedFunctions: "disallow" }]*/
+
+function doSomething() { }
+
+function doSomething() {
+    function doSomethingElse() { }
+}
+```
+
+:::
+
+Example of **correct** code for this rule with `{ blockScopedFunctions: "allow" }` option with `ecmaVersion: 2015`:
+
+::: correct { "sourceType": "script", "ecmaVersion": 2015 }
+
+```js
+/*eslint no-inner-declarations: ["error", "functions", { blockScopedFunctions: "allow" }]*/
+
+"use strict";
+
+if (test) {
+    function doSomething() { }
+}
+
+function doSomething() {
+    if (test) {
+        function doSomethingElse() { }
+    }
+}
+
+// OR
+
+function foo() {
+    "use strict";
+
+    if (test) {
+        function bar() { }
+    }
+}
+```
+
+:::
+
+`ESM modules` and both `class` declarations and expressions are always in strict mode.
+
+::: correct { "sourceType": "module" }
+
+```js
+/*eslint no-inner-declarations: ["error", "functions", { blockScopedFunctions: "allow" }]*/
+
+if (test) {
+    function doSomething() { }
+}
+
+function doSomethingElse() {
+    if (test) {
+        function doAnotherThing() { }
+    }
+}
+
+class Some {
+    static {
+        if (test) {
+            function doSomething() { }
+        }
+    }
+}
+
+const C = class {
+    static {
+        if (test) {
+            function doSomething() { }
+        }
+    }
+}
+```
+
+:::
+
 ## When Not To Use It
 
-The function declaration portion rule will be rendered obsolete when [block-scoped functions](https://bugzilla.mozilla.org/show_bug.cgi?id=585536) land in ES6, but until then, it should be left on to enforce valid constructions. Disable checking variable declarations when using [block-scoped-var](block-scoped-var) or if declaring variables in nested blocks is acceptable despite hoisting.
+By default, this rule disallows inner function declarations only in contexts where their behavior is unspecified and thus inconsistent (pre-ES6 environments) or legacy semantics apply (non-strict mode code). If your code targets pre-ES6 environments or is not in strict mode, you should enable this rule to prevent unexpected behavior.
+
+In ES6+ environments, in strict mode code, the behavior of inner function declarations is well-defined and consistent - they are always block-scoped. If your code targets only ES6+ environments and is in strict mode (ES modules, or code with `"use strict"` directives) then there is no need to enable this rule unless you want to disallow inner functions as a stylistic choice, in which case you should enable this rule with the option `blockScopedFunctions: "disallow"`.
+
+Disable checking variable declarations when using [block-scoped-var](block-scoped-var) or if declaring variables in nested blocks is acceptable despite hoisting.
diff --git a/lib/rules/no-inner-declarations.js b/lib/rules/no-inner-declarations.js
@@ -56,6 +56,15 @@ module.exports = {
         schema: [
             {
                 enum: ["functions", "both"]
+            },
+            {
+                type: "object",
+                properties: {
+                    blockScopedFunctions: {
+                        enum: ["allow", "disallow"]
+                    }
+                },
+                additionalProperties: false
             }
         ],
 
@@ -66,6 +75,10 @@ module.exports = {
 
     create(context) {
 
+        const sourceCode = context.sourceCode;
+        const ecmaVersion = context.languageOptions.ecmaVersion;
+        const blockScopedFunctions = context.options[1]?.blockScopedFunctions ?? "allow";
+
         /**
          * Ensure that a given node is at a program or function body's root.
          * @param {ASTNode} node Declaration node to check.
@@ -97,7 +110,15 @@ module.exports = {
 
         return {
 
-            FunctionDeclaration: check,
+            FunctionDeclaration(node) {
+                const isInStrictCode = sourceCode.getScope(node).upper.isStrict;
+
+                if (blockScopedFunctions === "allow" && ecmaVersion >= 2015 && isInStrictCode) {
+                    return;
+                }
+
+                check(node);
+            },
             VariableDeclaration(node) {
                 if (context.options[0] === "both" && node.kind === "var") {
                     check(node);
diff --git a/tests/lib/rules/no-inner-declarations.js b/tests/lib/rules/no-inner-declarations.js

-Original file line number
+Diff line change
             code: "class C { static { var x; } }",
             options: ["both"],
             languageOptions: { ecmaVersion: 2022 }
 +        },
 +        {
 +            code: "'use strict' \n if (test) { function doSomething() { } }",
 +            options: ["functions", { blockScopedFunctions: "allow" }],
 +            languageOptions: { ecmaVersion: 2022 }
 +        },
 +        {
 +            code: "'use strict' \n if (test) { function doSomething() { } }",
 +            options: ["functions"],
 +            languageOptions: { ecmaVersion: 2022 }
 +        },
 +        {
 +            code: "function foo() {'use strict' \n if (test) { function doSomething() { } } }",
 +            options: ["functions", { blockScopedFunctions: "allow" }],
 +            languageOptions: { ecmaVersion: 6 }
 +        },
 +        {
 +            code: "function foo() { { function bar() { } } }",
 +            options: ["functions", { blockScopedFunctions: "allow" }],
 +            languageOptions: { ecmaVersion: 2022, sourceType: "module" }
 +        },
 +        {
 +            code: "class C { method() { if(test) { function somethingElse() { } } } }",
 +            options: ["functions", { blockScopedFunctions: "allow" }],
 +            languageOptions: { ecmaVersion: 2022 }
 +        },
 +        {
 +            code: "const C = class { method() { if(test) { function somethingElse() { } } } }",
 +            options: ["functions", { blockScopedFunctions: "allow" }],
 +            languageOptions: { ecmaVersion: 2022 }
+        }
     ],
                 type: "VariableDeclaration"
             }]
         }, {
 -            code: "if (foo) function f(){ if(bar) var a; } ",
 +            code: "if (foo) function f(){ if(bar) var a; }",
             options: ["both"],
             errors: [{
                 messageId: "moveDeclToRoot",
             }]
         },
+        {
 -            code: "if (foo){ var a; }",
 +            code: "if (foo) { var a; }",
             options: ["both"],
             errors: [{
                 messageId: "moveDeclToRoot",
                 },
                 type: "VariableDeclaration"
             }]
 -        }, {
 +        },
 +        {
             code: "class C { method() { if(test) { var foo; } } }",
             options: ["both"],
             languageOptions: { ecmaVersion: 6 },
                 },
                 type: "VariableDeclaration"
             }]
 -        }, {
 -            code: "class C { static { if (test) { function foo() {} } } }",
 +        },
 +        {
 +            code: "class C { static { if (test) { var foo; } } }",
             options: ["both"],
             languageOptions: { ecmaVersion: 2022 },
 +            errors: [{
 +                messageId: "moveDeclToRoot",
 +                data: {
 +                    type: "variable",
 +                    body: "class static block body"
 +                },
 +                type: "VariableDeclaration"
 +            }]
 +        },
 +        {
 +            code: "class C { static { if (test) { function foo() {} } } }",
 +            options: ["both", { blockScopedFunctions: "disallow" }],
 +            languageOptions: { ecmaVersion: 2022 },
             errors: [{
                 messageId: "moveDeclToRoot",
                 data: {
                 },
                 type: "FunctionDeclaration"
             }]
 -        }, {
 -            code: "class C { static { if (test) { var foo; } } }",
 +        },
 +        {
 +            code: "class C { static { if (test) { if (anotherTest) { var foo; } } } }",
             options: ["both"],
             languageOptions: { ecmaVersion: 2022 },
             errors: [{
                 },
                 type: "VariableDeclaration"
             }]
 -        }, {
 -            code: "class C { static { if (test) { if (anotherTest) { var foo; } } } }",
 +        },
 +        {
 +            code: "if (test) { function doSomething() { } }",
 +            options: ["both", { blockScopedFunctions: "allow" }],
 +            languageOptions: { ecmaVersion: 5 },
 +            errors: [{
 +                messageId: "moveDeclToRoot",
 +                data: {
 +                    type: "function",
 +                    body: "program"
 +                },
 +                type: "FunctionDeclaration"
 +            }]
 +        },
 +        {
 +            code: "if (test) { function doSomething() { } }",
 +            options: ["both", { blockScopedFunctions: "disallow" }],
 +            languageOptions: { ecmaVersion: 2022 },
 +            errors: [{
 +                messageId: "moveDeclToRoot",
 +                data: {
 +                    type: "function",
 +                    body: "program"
 +                },
 +                type: "FunctionDeclaration"
 +            }]
 +        },
 +        {
 +            code: "'use strict' \n if (test) { function doSomething() { } }",
 +            options: ["both", { blockScopedFunctions: "disallow" }],
 +            languageOptions: { ecmaVersion: 2022 },
 +            errors: [{
 +                messageId: "moveDeclToRoot",
 +                data: {
 +                    type: "function",
 +                    body: "program"
 +                },
 +                type: "FunctionDeclaration"
 +            }]
 +        },
 +        {
 +            code: "'use strict' \n if (test) { function doSomething() { } }",
 +            options: ["both", { blockScopedFunctions: "disallow" }],
 +            languageOptions: { ecmaVersion: 5 },
 +            errors: [{
 +                messageId: "moveDeclToRoot",
 +                data: {
 +                    type: "function",
 +                    body: "program"
 +                },
 +                type: "FunctionDeclaration"
 +            }]
 +        },
 +        {
 +            code: "'use strict' \n if (test) { function doSomething() { } }",
 +            options: ["both", { blockScopedFunctions: "allow" }],
 +            languageOptions: { ecmaVersion: 5 },
 +            errors: [{
 +                messageId: "moveDeclToRoot",
 +                data: {
 +                    type: "function",
 +                    body: "program"
 +                },
 +                type: "FunctionDeclaration"
 +            }]
 +        },
 +        {
 +            code: "function foo() {'use strict' \n { function bar() { } } }",
 +            options: ["both", { blockScopedFunctions: "disallow" }],
 +            languageOptions: { ecmaVersion: 2022 },
 +            errors: [{
 +                messageId: "moveDeclToRoot",
 +                data: {
 +                    type: "function",
 +                    body: "function body"
 +                },
 +                type: "FunctionDeclaration"
 +            }]
 +        },
 +        {
 +            code: "function foo() {'use strict' \n { function bar() { } } }",
 +            options: ["both", { blockScopedFunctions: "disallow" }],
 +            languageOptions: { ecmaVersion: 5 },
 +            errors: [{
 +                messageId: "moveDeclToRoot",
 +                data: {
 +                    type: "function",
 +                    body: "function body"
 +                },
 +                type: "FunctionDeclaration"
 +            }]
 +        },
 +        {
 +            code: "function doSomething() { 'use strict' \n do { function somethingElse() { } } while (test); }",
 +            options: ["both", { blockScopedFunctions: "disallow" }],
 +            languageOptions: { ecmaVersion: 5 },
 +            errors: [{
 +                messageId: "moveDeclToRoot",
 +                data: {
 +                    type: "function",
 +                    body: "function body"
 +                },
 +                type: "FunctionDeclaration"
 +            }]
 +        },
 +        {
 +            code: "{ function foo () {'use strict' \n console.log('foo called'); } }",
             options: ["both"],
             languageOptions: { ecmaVersion: 2022 },
             errors: [{
                 messageId: "moveDeclToRoot",
                 data: {
 -                    type: "variable",
 -                    body: "class static block body"
 +                    type: "function",
 +                    body: "program"
                 },
 -                type: "VariableDeclaration"
 +                type: "FunctionDeclaration"
             }]
+        }
+    ]