Skip to content

Commit

Permalink
feat!: no-inner-declaration new default behaviour and option (#17885)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Tanujkanti4441 authored Jan 9, 2024
1 parent b4e0503 commit 701f1af
Show file tree
Hide file tree
Showing 3 changed files with 334 additions and 23 deletions.
159 changes: 149 additions & 10 deletions docs/src/rules/no-inner-declarations.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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() { }
}
Expand All @@ -90,14 +107,6 @@ function doSomethingElse() {
}

if (foo) function f(){}

class C {
static {
if (test) {
function doSomething() { }
}
}
}
```

:::
Expand All @@ -115,6 +124,14 @@ function doSomethingElse() {
function doAnotherThing() { }
}

function doSomethingElse() {
"use strict";

if (test) {
function doAnotherThing() { }
}
}

class C {
static {
function doSomething() { }
Expand Down Expand Up @@ -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.
23 changes: 22 additions & 1 deletion lib/rules/no-inner-declarations.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ module.exports = {
schema: [
{
enum: ["functions", "both"]
},
{
type: "object",
properties: {
blockScopedFunctions: {
enum: ["allow", "disallow"]
}
},
additionalProperties: false
}
],

Expand All @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
Loading

0 comments on commit 701f1af

Please sign in to comment.