-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
eslint-plugin: Add rule
no-unused-vars-before-return
(#12828)
* scripts: Bump ESLint dependency to 5.10.x * eslint-plugin: Add rule `no-unused-vars-before-return` * eslint-plugin: Rephrase return-unused message for declarator * Update error message in the unit test to reflect code changes * Scripts: Include Migration Guide link for ESLint major bump CHANGELOG note
- Loading branch information
Showing
11 changed files
with
364 additions
and
182 deletions.
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
31 changes: 31 additions & 0 deletions
31
packages/eslint-plugin/docs/rules/no-unused-vars-before-return.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
# Avoid assigning variable values if unused before a return (no-unused-vars-before-return) | ||
|
||
To avoid wastefully computing the result of a function call when assigning a variable value, the variable should be declared as late as possible. In practice, if a function includes an early return path, any variable declared prior to the `return` must be referenced at least once. Otherwise, in the condition which satisfies that return path, the declared variable is effectively considered dead code. This can have a performance impact when the logic performed in assigning the value is non-trivial. | ||
|
||
## Rule details | ||
|
||
The following patterns are considered warnings: | ||
|
||
```js | ||
function example( number ) { | ||
const foo = doSomeCostlyOperation(); | ||
if ( number > 10 ) { | ||
return number + 1; | ||
} | ||
|
||
return number + foo; | ||
} | ||
``` | ||
|
||
The following patterns are not considered warnings: | ||
|
||
```js | ||
function example( number ) { | ||
if ( number > 10 ) { | ||
return number + 1; | ||
} | ||
|
||
const foo = doSomeCostlyOperation(); | ||
return number + foo; | ||
} | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
module.exports = { | ||
configs: require( './configs' ), | ||
rules: require( './rules' ), | ||
}; |
45 changes: 45 additions & 0 deletions
45
packages/eslint-plugin/rules/__tests__/no-unused-vars-before-return.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { RuleTester } from 'eslint'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import rule from '../no-unused-vars-before-return'; | ||
|
||
const ruleTester = new RuleTester( { | ||
parserOptions: { | ||
ecmaVersion: 6, | ||
}, | ||
} ); | ||
|
||
ruleTester.run( 'no-unused-vars-before-return', rule, { | ||
valid: [ | ||
{ | ||
code: ` | ||
function example( number ) { | ||
if ( number > 10 ) { | ||
return number + 1; | ||
} | ||
const foo = doSomeCostlyOperation(); | ||
return number + foo; | ||
}`, | ||
}, | ||
], | ||
invalid: [ | ||
{ | ||
code: ` | ||
function example( number ) { | ||
const foo = doSomeCostlyOperation(); | ||
if ( number > 10 ) { | ||
return number + 1; | ||
} | ||
return number + foo; | ||
}`, | ||
errors: [ { message: 'Variables should not be assigned until just prior its first reference. An early return statement may leave this variable unused.' } ], | ||
}, | ||
], | ||
} ); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
module.exports = require( 'requireindex' )( __dirname ); |
56 changes: 56 additions & 0 deletions
56
packages/eslint-plugin/rules/no-unused-vars-before-return.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
module.exports = { | ||
meta: { | ||
type: 'problem', | ||
schema: [], | ||
}, | ||
create( context ) { | ||
return { | ||
ReturnStatement( node ) { | ||
let functionScope = context.getScope(); | ||
while ( functionScope.type !== 'function' && functionScope.upper ) { | ||
functionScope = functionScope.upper; | ||
} | ||
|
||
if ( ! functionScope ) { | ||
return; | ||
} | ||
|
||
for ( const variable of functionScope.variables ) { | ||
const declaratorCandidate = variable.defs.find( ( def ) => { | ||
return ( | ||
def.node.type === 'VariableDeclarator' && | ||
// Allow declarations which are not initialized. | ||
def.node.init && | ||
// Target function calls as "expensive". | ||
def.node.init.type === 'CallExpression' && | ||
// Allow unused if part of an object destructuring. | ||
def.node.id.type !== 'ObjectPattern' && | ||
// Only target assignments preceding `return`. | ||
def.node.end < node.end | ||
); | ||
} ); | ||
|
||
if ( ! declaratorCandidate ) { | ||
continue; | ||
} | ||
|
||
// The first entry in `references` is the declaration | ||
// itself, which can be ignored. | ||
const isUsedBeforeReturn = variable.references.slice( 1 ).some( ( reference ) => { | ||
return reference.identifier.end < node.end; | ||
} ); | ||
|
||
if ( isUsedBeforeReturn ) { | ||
continue; | ||
} | ||
|
||
context.report( | ||
declaratorCandidate.node, | ||
'Variables should not be assigned until just prior its first reference. ' + | ||
'An early return statement may leave this variable unused.' | ||
); | ||
} | ||
}, | ||
}; | ||
}, | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters