Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update: allowFunctionParams option in no-underscore-dangle (fixes 12579) #13545

Merged
merged 20 commits into from Aug 14, 2020
Merged

Update: allowFunctionParams option in no-underscore-dangle (fixes 12579) #13545

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
6f7c222
#12579 add allowFunctionParams option
sunghyunjo Aug 1, 2020
ad72c3f
#12579 edit doc & function name
sunghyunjo Aug 2, 2020
0951da6
#12579 add test case
sunghyunjo Aug 2, 2020
a5d7370
#12579 add allowFunctionParam rule in docs
sunghyunjo Aug 2, 2020
1d12aea
#12579 Update : test case when option is false
sunghyunjo Aug 2, 2020
5109bf0
Return to origin code
sunghyunjo Aug 2, 2020
03689ba
Remove comments
sunghyunjo Aug 2, 2020
b1ef8db
Edit what was reviewed
sunghyunjo Aug 2, 2020
1703293
Update: Destructuring param test
Aug 3, 2020
4f1b8cf
Update: invalid test case
Aug 3, 2020
fbdc7fd
Change initial state to true
sunghyunjo Aug 3, 2020
64911e9
#12579 Update: Edit what was reviewed
Aug 4, 2020
a849734
Merge branch 'issue/#12579' of https://github.com/sunghyunjo/eslint i…
Aug 4, 2020
c50c6d5
#12579 Fix a typo
sunghyunjo Aug 4, 2020
7e9ccea
#12579 Update : allow option
sunghyunjo Aug 4, 2020
850c80f
#12579 Update: Edit what was reviewed
Aug 7, 2020
7423641
#12579 Update : check type of param
sunghyunjo Aug 9, 2020
7667054
#12579 Simplify the code
sunghyunjo Aug 9, 2020
1da8418
Update : test case
sunghyunjo Aug 13, 2020
39f89e2
Fix : ...bar -> ..._bar
sunghyunjo Aug 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 23 additions & 5 deletions docs/rules/no-underscore-dangle.md
View file
Open in desktop
Expand Up @@ -33,17 +33,21 @@ var _ = require('underscore');
var obj = _.contains(items, item);
obj.__proto__ = {};
var file = __filename;
function foo(_bar) {};
const foo = { onClick(_bar) {} };
const foo = (_bar) => {};
```

## Options

This rule has an object option:

* `"allow"` allows specified identifiers to have dangling underscores
* `"allowAfterThis": false` (default) disallows dangling underscores in members of the `this` object
* `"allowAfterSuper": false` (default) disallows dangling underscores in members of the `super` object
* `"allowAfterThisConstructor": false` (default) disallows dangling underscores in members of the `this.constructor` object
* `"enforceInMethodNames": false` (default) allows dangling underscores in method names
- `"allow"` allows specified identifiers to have dangling underscores
- `"allowAfterThis": false` (default) disallows dangling underscores in members of the `this` object
- `"allowAfterSuper": false` (default) disallows dangling underscores in members of the `super` object
- `"allowAfterThisConstructor": false` (default) disallows dangling underscores in members of the `this.constructor` object
- `"enforceInMethodNames": false` (default) allows dangling underscores in method names
- `"allowFunctionParams": true` (default) allows dangling underscores in function parameter names
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make more sense, it should defaults to false. we can change it in next major release( as it's a breaking change). 😄


### allow

Expand Down Expand Up @@ -113,6 +117,20 @@ const o = {
};
```

### allowFunctionParams

Examples of **incorrect** code for this rule with the `{ "allowFunctionParams": false }` option:

```js
/*eslint no-underscore-dangle: ["error", { "allowFunctionParams": false }]*/

function foo (_bar) {}

const foo = function onClick (_bar) {}

const foo = (_bar) => {};
sunghyunjo marked this conversation as resolved.
Show resolved Hide resolved
```

## When Not To Use It

If you want to allow dangling underscores in identifiers, then you can safely turn this rule off.
74 changes: 53 additions & 21 deletions lib/rules/no-underscore-dangle.js
View file
Open in desktop
@@ -1,5 +1,5 @@
/**
* @fileoverview Rule to flag trailing underscores in variable declarations.
* @fileoverview Rule to flag dangling underscores in variable declarations.
* @author Matt DuVall <http://www.mattduvall.com>
*/

Expand Down Expand Up @@ -45,6 +45,10 @@ module.exports = {
enforceInMethodNames: {
type: "boolean",
default: false
},
allowFunctionParams: {
type: "boolean",
default: true
}
},
additionalProperties: false
Expand All @@ -64,6 +68,7 @@ module.exports = {
const allowAfterSuper = typeof options.allowAfterSuper !== "undefined" ? options.allowAfterSuper : false;
const allowAfterThisConstructor = typeof options.allowAfterThisConstructor !== "undefined" ? options.allowAfterThisConstructor : false;
const enforceInMethodNames = typeof options.enforceInMethodNames !== "undefined" ? options.enforceInMethodNames : false;
const allowFunctionParams = typeof options.allowFunctionParams !== "undefined" ? options.allowFunctionParams : true;

//-------------------------------------------------------------------------
// Helpers
Expand All @@ -80,12 +85,12 @@ module.exports = {
}

/**
* Check if identifier has a underscore at the end
* Check if identifier has a dangling underscore
* @param {string} identifier name of the node
* @returns {boolean} true if its is present
* @private
*/
function hasTrailingUnderscore(identifier) {
function hasDanglingUnderscore(identifier) {
const len = identifier.length;

return identifier !== "_" && (identifier[0] === "_" || identifier[len - 1] === "_");
Expand Down Expand Up @@ -126,16 +131,40 @@ module.exports = {
}

/**
* Check if function has a underscore at the end
* Check if function parameter has a dangling underscore.
* @param {ASTNode} node function node to evaluate
* @returns {void}
* @private
*/
function checkForDanglingUnderscoreInFunctionParameters(node) {
if (!allowFunctionParams) {
node.params.forEach(param => {
const identifier = param.name;
sunghyunjo marked this conversation as resolved.
Show resolved Hide resolved

if (typeof identifier !== "undefined" && hasDanglingUnderscore(identifier) && !isAllowed(identifier)) {
context.report({
node: param,
messageId: "unexpectedUnderscore",
data: {
identifier
}
});
}
});
}
}

/**
* Check if function has a dangling underscore
* @param {ASTNode} node node to evaluate
* @returns {void}
* @private
*/
function checkForTrailingUnderscoreInFunctionDeclaration(node) {
if (node.id) {
function checkForDanglingUnderscoreInFunction(node) {
if (node.type === "FunctionDeclaration" && node.id) {
const identifier = node.id.name;

if (typeof identifier !== "undefined" && hasTrailingUnderscore(identifier) && !isAllowed(identifier)) {
if (typeof identifier !== "undefined" && hasDanglingUnderscore(identifier) && !isAllowed(identifier)) {
context.report({
node,
messageId: "unexpectedUnderscore",
Expand All @@ -145,18 +174,19 @@ module.exports = {
});
}
}
checkForDanglingUnderscoreInFunctionParameters(node);
}

/**
* Check if variable expression has a underscore at the end
* Check if variable expression has a dangling underscore
* @param {ASTNode} node node to evaluate
* @returns {void}
* @private
*/
function checkForTrailingUnderscoreInVariableExpression(node) {
function checkForDanglingUnderscoreInVariableExpression(node) {
const identifier = node.id.name;

if (typeof identifier !== "undefined" && hasTrailingUnderscore(identifier) &&
if (typeof identifier !== "undefined" && hasDanglingUnderscore(identifier) &&
!isSpecialCaseIdentifierInVariableExpression(identifier) && !isAllowed(identifier)) {
context.report({
node,
Expand All @@ -169,18 +199,18 @@ module.exports = {
}

/**
* Check if member expression has a underscore at the end
* Check if member expression has a dangling underscore
* @param {ASTNode} node node to evaluate
* @returns {void}
* @private
*/
function checkForTrailingUnderscoreInMemberExpression(node) {
function checkForDanglingUnderscoreInMemberExpression(node) {
const identifier = node.property.name,
isMemberOfThis = node.object.type === "ThisExpression",
isMemberOfSuper = node.object.type === "Super",
isMemberOfThisConstructor = isThisConstructorReference(node);

if (typeof identifier !== "undefined" && hasTrailingUnderscore(identifier) &&
if (typeof identifier !== "undefined" && hasDanglingUnderscore(identifier) &&
!(isMemberOfThis && allowAfterThis) &&
!(isMemberOfSuper && allowAfterSuper) &&
!(isMemberOfThisConstructor && allowAfterThisConstructor) &&
Expand All @@ -196,16 +226,16 @@ module.exports = {
}

/**
* Check if method declaration or method property has a underscore at the end
* Check if method declaration or method property has a dangling underscore
* @param {ASTNode} node node to evaluate
* @returns {void}
* @private
*/
function checkForTrailingUnderscoreInMethod(node) {
function checkForDanglingUnderscoreInMethod(node) {
const identifier = node.key.name;
const isMethod = node.type === "MethodDefinition" || node.type === "Property" && node.method;

if (typeof identifier !== "undefined" && enforceInMethodNames && isMethod && hasTrailingUnderscore(identifier) && !isAllowed(identifier)) {
if (typeof identifier !== "undefined" && enforceInMethodNames && isMethod && hasDanglingUnderscore(identifier) && !isAllowed(identifier)) {
context.report({
node,
messageId: "unexpectedUnderscore",
Expand All @@ -221,11 +251,13 @@ module.exports = {
//--------------------------------------------------------------------------

return {
FunctionDeclaration: checkForTrailingUnderscoreInFunctionDeclaration,
VariableDeclarator: checkForTrailingUnderscoreInVariableExpression,
MemberExpression: checkForTrailingUnderscoreInMemberExpression,
MethodDefinition: checkForTrailingUnderscoreInMethod,
Property: checkForTrailingUnderscoreInMethod
FunctionDeclaration: checkForDanglingUnderscoreInFunction,
VariableDeclarator: checkForDanglingUnderscoreInVariableExpression,
MemberExpression: checkForDanglingUnderscoreInMemberExpression,
MethodDefinition: checkForDanglingUnderscoreInMethod,
Property: checkForDanglingUnderscoreInMethod,
FunctionExpression: checkForDanglingUnderscoreInFunction,
ArrowFunctionExpression: checkForDanglingUnderscoreInFunction
};

}
Expand Down
31 changes: 29 additions & 2 deletions tests/lib/rules/no-underscore-dangle.js
View file
Open in desktop
Expand Up @@ -26,6 +26,12 @@ ruleTester.run("no-underscore-dangle", rule, {
"console.log(__filename); console.log(__dirname);",
"var _ = require('underscore');",
"var a = b._;",
"function foo(_bar) {}",
sunghyunjo marked this conversation as resolved.
Show resolved Hide resolved
{ code: "function foo( _bar = 0) {}", parserOptions: { ecmaVersion: 6 } },
{ code: "const foo = { onClick(_bar) { } }", parserOptions: { ecmaVersion: 6 } },
{ code: "const foo = { onClick(_bar = 0) { } }", parserOptions: { ecmaVersion: 6 } },
{ code: "const foo = (_bar) => {}", parserOptions: { ecmaVersion: 6 } },
{ code: "const foo = (_bar = 0) => {}", parserOptions: { ecmaVersion: 6 } },
{ code: "export default function() {}", parserOptions: { ecmaVersion: 6, sourceType: "module" } },
{ code: "var _foo = 1", options: [{ allow: ["_foo"] }] },
{ code: "var __proto__ = 1;", options: [{ allow: ["__proto__"] }] },
Expand All @@ -40,7 +46,19 @@ ruleTester.run("no-underscore-dangle", rule, {
{ code: "const o = { _onClick() { } }", options: [{ allow: ["_onClick"], enforceInMethodNames: true }], parserOptions: { ecmaVersion: 6 } },
{ code: "const o = { _foo: 'bar' }", parserOptions: { ecmaVersion: 6 } },
{ code: "const o = { foo_: 'bar' }", parserOptions: { ecmaVersion: 6 } },
{ code: "this.constructor._bar", options: [{ allowAfterThisConstructor: true }] }
{ code: "this.constructor._bar", options: [{ allowAfterThisConstructor: true }] },
{ code: "const foo = { onClick(bar) { } }", parserOptions: { ecmaVersion: 6 } },
{ code: "const foo = (bar) => {}", parserOptions: { ecmaVersion: 6 } },
{ code: "function foo(_bar) {}", options: [{ allowFunctionParams: true }] },
{ code: "function foo( _bar = 0) {}", options: [{ allowFunctionParams: true }], parserOptions: { ecmaVersion: 6 } },
{ code: "const foo = { onClick(_bar) { } }", options: [{ allowFunctionParams: true }], parserOptions: { ecmaVersion: 6 } },
{ code: "const foo = (_bar) => {}", options: [{ allowFunctionParams: true }], parserOptions: { ecmaVersion: 6 } },
{ code: "function foo(bar) {}", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 } },
{ code: "const foo = { onClick(bar) { } }", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 } },
{ code: "const foo = (bar) => {}", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 } },
{ code: "function foo(_bar) {}", options: [{ allowFunctionParams: false, allow: ["_bar"] }] },
{ code: "const foo = { onClick(_bar) { } }", options: [{ allowFunctionParams: false, allow: ["_bar"] }], parserOptions: { ecmaVersion: 6 } },
{ code: "const foo = (_bar) => {}", options: [{ allowFunctionParams: false, allow: ["_bar"] }], parserOptions: { ecmaVersion: 6 } }
],
invalid: [
{ code: "var _foo = 1", errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_foo" }, type: "VariableDeclarator" }] },
Expand All @@ -57,6 +75,15 @@ ruleTester.run("no-underscore-dangle", rule, {
{ code: "const o = { _onClick() { } }", options: [{ enforceInMethodNames: true }], parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_onClick" }, type: "Property" }] },
{ code: "const o = { onClick_() { } }", options: [{ enforceInMethodNames: true }], parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "onClick_" }, type: "Property" }] },
{ code: "this.constructor._bar", errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_bar" }, type: "MemberExpression" }] },
{ code: "foo.constructor._bar", options: [{ allowAfterThisConstructor: true }], errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_bar" }, type: "MemberExpression" }] }
{ code: "function foo(_bar) {}", options: [{ allowFunctionParams: false }], errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_bar" }, type: "Identifier" }] },
{ code: "const foo = { onClick(_bar) { } }", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_bar" }, type: "Identifier" }] },
sunghyunjo marked this conversation as resolved.
Show resolved Hide resolved
{ code: "const foo = (_bar) => {}", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_bar" }, type: "Identifier" }] }

/*
* { code: "function foo(_bar = 0) {}", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_bar" }, type: "Identifier" }] },
* { code: "const foo = { onClick(_bar = 0) { } }", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_bar" }, type: "Identifier" }] },
* { code: "const foo = (_bar = 0) => {}", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_bar" }, type: "Identifier" }] }
*/

]
});