From b46f3ee0dae4add9df99cae940b641ad8de58b9e Mon Sep 17 00:00:00 2001 From: Sunghyun Cho Date: Sat, 15 Aug 2020 05:49:57 +0900 Subject: [PATCH] Update: allowFunctionParams option in no-underscore-dangle (fixes 12579) (#13545) * #12579 add allowFunctionParams option * #12579 edit doc & function name * #12579 add test case * #12579 add allowFunctionParam rule in docs * #12579 Update : test case when option is false * Return to origin code * Remove comments * Edit what was reviewed * Update: Destructuring param test * Update: invalid test case * Change initial state to true * #12579 Update: Edit what was reviewed * #12579 Fix a typo * #12579 Update : allow option * #12579 Update: Edit what was reviewed * #12579 Update : check type of param * #12579 Simplify the code * Update : test case * Fix : ...bar -> ..._bar --- docs/rules/no-underscore-dangle.md | 34 ++++++++-- lib/rules/no-underscore-dangle.js | 87 +++++++++++++++++++------ tests/lib/rules/no-underscore-dangle.js | 45 ++++++++++++- 3 files changed, 138 insertions(+), 28 deletions(-) diff --git a/docs/rules/no-underscore-dangle.md b/docs/rules/no-underscore-dangle.md index ebe7fd7b6d0..ded42815d2a 100644 --- a/docs/rules/no-underscore-dangle.md +++ b/docs/rules/no-underscore-dangle.md @@ -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 ### allow @@ -113,6 +117,26 @@ 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) {} +function foo (_bar = 0) {} +function foo (..._bar) {} + +const foo = function onClick (_bar) {} +const foo = function onClick (_bar = 0) {} +const foo = function onClick (..._bar) {} + +const foo = (_bar) => {}; +const foo = (_bar = 0) => {}; +const foo = (..._bar) => {}; +``` + ## When Not To Use It If you want to allow dangling underscores in identifiers, then you can safely turn this rule off. diff --git a/lib/rules/no-underscore-dangle.js b/lib/rules/no-underscore-dangle.js index cac594e1004..87d2336fa4a 100644 --- a/lib/rules/no-underscore-dangle.js +++ b/lib/rules/no-underscore-dangle.js @@ -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 */ @@ -45,6 +45,10 @@ module.exports = { enforceInMethodNames: { type: "boolean", default: false + }, + allowFunctionParams: { + type: "boolean", + default: true } }, additionalProperties: false @@ -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 @@ -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] === "_"); @@ -126,16 +131,53 @@ 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 { type } = param; + let nodeToCheck; + + if (type === "RestElement") { + nodeToCheck = param.argument; + } else if (type === "AssignmentPattern") { + nodeToCheck = param.left; + } else { + nodeToCheck = param; + } + + if (nodeToCheck.type === "Identifier") { + const identifier = nodeToCheck.name; + + if (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", @@ -145,18 +187,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, @@ -169,18 +212,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) && @@ -196,16 +239,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", @@ -221,11 +264,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 }; } diff --git a/tests/lib/rules/no-underscore-dangle.js b/tests/lib/rules/no-underscore-dangle.js index 4baef3e82da..89f20835de2 100644 --- a/tests/lib/rules/no-underscore-dangle.js +++ b/tests/lib/rules/no-underscore-dangle.js @@ -26,6 +26,18 @@ ruleTester.run("no-underscore-dangle", rule, { "console.log(__filename); console.log(__dirname);", "var _ = require('underscore');", "var a = b._;", + "function foo(_bar) {}", + "function foo(bar_) {}", + "(function _foo() {})", + { code: "function foo(_bar) {}", options: [{}] }, + { 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: "function foo( ..._bar) {}", parserOptions: { ecmaVersion: 6 } }, + { code: "const foo = (..._bar) => {}", parserOptions: { ecmaVersion: 6 } }, + { code: "const foo = { onClick(..._bar) { } }", 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__"] }] }, @@ -40,7 +52,24 @@ 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 } }, + { code: "function foo([_bar]) {}", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 } }, + { code: "function foo([_bar] = []) {}", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 } }, + { code: "function foo( { _bar }) {}", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 } }, + { code: "function foo( { _bar = 0 } = {}) {}", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 } }, + { code: "function foo(...[_bar]) {}", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 2016 } } ], invalid: [ { code: "var _foo = 1", errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_foo" }, type: "VariableDeclarator" }] }, @@ -57,6 +86,18 @@ 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: "(function foo(_bar) {})", options: [{ allowFunctionParams: false }], errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_bar" }, type: "Identifier" }] }, + { code: "function foo(bar, _foo) {}", options: [{ allowFunctionParams: false }], errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_foo" }, type: "Identifier" }] }, + { code: "const foo = { onClick(_bar) { } }", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_bar" }, type: "Identifier" }] }, + { 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: "AssignmentPattern" }] }, + { code: "const foo = { onClick(_bar = 0) { } }", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_bar" }, type: "AssignmentPattern" }] }, + { code: "const foo = (_bar = 0) => {}", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_bar" }, type: "AssignmentPattern" }] }, + { code: "function foo(..._bar) {}", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_bar" }, type: "RestElement" }] }, + { code: "const foo = { onClick(..._bar) { } }", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_bar" }, type: "RestElement" }] }, + { code: "const foo = (..._bar) => {}", options: [{ allowFunctionParams: false }], parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "unexpectedUnderscore", data: { identifier: "_bar" }, type: "RestElement" }] } + + ] });