From d76824057835bf3e5d1b21d7308af0ab46bbe778 Mon Sep 17 00:00:00 2001 From: pfg Date: Mon, 11 Feb 2019 17:21:47 -0800 Subject: [PATCH 1/4] Update: Add never option for new-parens (fixes #10034) --- docs/rules/new-parens.md | 38 ++++++++++++++++++--- lib/rules/new-parens.js | 50 +++++++++++++++++++++++----- tests/lib/rules/new-parens.js | 62 ++++++++++++++++++++++++++++++++++- 3 files changed, 136 insertions(+), 14 deletions(-) diff --git a/docs/rules/new-parens.md b/docs/rules/new-parens.md index 85038a62dd8..56ca2a231ad 100644 --- a/docs/rules/new-parens.md +++ b/docs/rules/new-parens.md @@ -1,4 +1,4 @@ -# require parentheses when invoking a constructor with no arguments (new-parens) +# Require parentheses when invoking a constructor with no arguments (new-parens) JavaScript allows the omission of parentheses when invoking a function via the `new` keyword and the constructor has no arguments. However, some coders believe that omitting the parentheses is inconsistent with the rest of the language and thus makes code less clear. @@ -8,9 +8,18 @@ var person = new Person; ## Rule Details -This rule requires parentheses when invoking a constructor with no arguments using the `new` keyword in order to increase code clarity. +This rule can enforce or disallow parentheses when invoking a constructor with no arguments using the `new` keyword. -Examples of **incorrect** code for this rule: +## Options + +This rule takes one option. + +- `"always"` enforces parenthesis after a new constructor with no arguments (default) +- `"never"` enforces no parenthesis after a new constructor with no arguments + +### always + +Examples of **incorrect** code for this rule with the `"always"` option: ```js /*eslint new-parens: "error"*/ @@ -19,7 +28,18 @@ var person = new Person; var person = new (Person); ``` -Examples of **correct** code for this rule: +Examples of **correct** code for this rule with the `"always"` option: + +```js +/*eslint new-parens: "error"*/ + +var person = new Person(); +var person = new (Person)(); +``` + +### never + +Examples of **incorrect** code for this rule with the `"never"` option: ```js /*eslint new-parens: "error"*/ @@ -27,3 +47,13 @@ Examples of **correct** code for this rule: var person = new Person(); var person = new (Person)(); ``` + +Examples of **correct** code for this rule with the `"never"` option: + +```js +/*eslint new-parens: "error"*/ + +var person = new Person; +var person = (new Person); +var person = new Person("Name"); +``` diff --git a/lib/rules/new-parens.js b/lib/rules/new-parens.js index edd3c1e1c2e..60adc1eca40 100644 --- a/lib/rules/new-parens.js +++ b/lib/rules/new-parens.js @@ -31,31 +31,63 @@ module.exports = { }, fixable: "code", - schema: [], + schema: { + anyOf: [ + { + type: "array", + items: [ + { + enum: ["always", "never"] + } + ], + minItems: 0, + maxItems: 1 + } + ] + }, messages: { - missing: "Missing '()' invoking a constructor." + missing: "Missing '()' invoking a constructor.", + using: "'()' included invoking a constructor with no arguments." } }, create(context) { + const options = context.options; + const always = options[0] !== "never"; // Default is never + const sourceCode = context.getSourceCode(); return { NewExpression(node) { if (node.arguments.length !== 0) { - return; // shortcut: if there are arguments, there have to be parens + return; // if there are arguments, there have to be parens } const lastToken = sourceCode.getLastToken(node); const hasLastParen = lastToken && astUtils.isClosingParenToken(lastToken); const hasParens = hasLastParen && astUtils.isOpeningParenToken(sourceCode.getTokenBefore(lastToken)); - if (!hasParens) { - context.report({ - node, - messageId: "missing", - fix: fixer => fixer.insertTextAfter(node, "()") - }); + if (always) { + if (!hasParens) { + context.report({ + node, + messageId: "missing", + fix: fixer => fixer.insertTextAfter(node, "()") + }); + } + } else { + if (hasParens) { + context.report({ + node, + messageId: "using", + fix: fixer => [ + fixer.remove(sourceCode.getTokenBefore(lastToken)), + fixer.remove(lastToken), + fixer.insertTextBefore(node, "("), + fixer.insertTextAfter(node, ")") + ] + }); + } } } }; diff --git a/tests/lib/rules/new-parens.js b/tests/lib/rules/new-parens.js index d56cd079d44..521f3d12619 100644 --- a/tests/lib/rules/new-parens.js +++ b/tests/lib/rules/new-parens.js @@ -17,6 +17,7 @@ const parser = require("../../fixtures/fixture-parser"), // Tests //------------------------------------------------------------------------------ const error = { messageId: "missing", type: "NewExpression" }; +const neverError = { messageId: "using", type: "NewExpression" }; const ruleTester = new RuleTester(); @@ -29,7 +30,16 @@ ruleTester.run("new-parens", rule, { "var a = (new Date());", "var a = new foo.Bar();", "var a = (new Foo()).bar;", - { code: "new Storage('state');", parser: parser("typescript-parsers/new-parens") } + { code: "new Storage('state');", parser: parser("typescript-parsers/new-parens") }, + + // Never + { code: "var a = new Date;", options: ["never"] }, + { code: "var a = new Date(function() {});", options: ["never"] }, + { code: "var a = new (Date);", options: ["never"] }, + { code: "var a = new ((Date));", options: ["never"] }, + { code: "var a = (new Date);", options: ["never"] }, + { code: "var a = new foo.Bar;", options: ["never"] }, + { code: "var a = (new Foo).bar;", options: ["never"] } ], invalid: [ { @@ -73,6 +83,56 @@ ruleTester.run("new-parens", rule, { code: "var a = (new Foo).bar;", output: "var a = (new Foo()).bar;", errors: [error] + }, + + // Never + { + code: "var a = new Date();", + output: "var a = (new Date);", + options: ["never"], + errors: [neverError] + }, + { + code: "var a = new Date()", + output: "var a = (new Date)", + options: ["never"], + errors: [neverError] + }, + { + code: "var a = new (Date)();", + output: "var a = (new (Date));", + options: ["never"], + errors: [neverError] + }, + { + code: "var a = new (Date)()", + output: "var a = (new (Date))", + options: ["never"], + errors: [neverError] + }, + { + code: "var a = (new Date())", + output: "var a = ((new Date))", + options: ["never"], + errors: [neverError] + }, + { + code: "var a = (new Date())()", + output: "var a = ((new Date))()", + options: ["never"], + errors: [neverError] + }, + { + code: "var a = new foo.Bar();", + output: "var a = (new foo.Bar);", + options: ["never"], + errors: [neverError] + }, + { + code: "var a = (new Foo()).bar;", + output: "var a = ((new Foo)).bar;", + options: ["never"], + errors: [neverError] } ] }); From f2db326ce05a466758ff560ee12f4bda156756fe Mon Sep 17 00:00:00 2001 From: pfg Date: Mon, 11 Feb 2019 17:38:56 -0800 Subject: [PATCH 2/4] Docs: Fix eslint comment in new-parens never example --- docs/rules/new-parens.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/rules/new-parens.md b/docs/rules/new-parens.md index 56ca2a231ad..1640d37a2e4 100644 --- a/docs/rules/new-parens.md +++ b/docs/rules/new-parens.md @@ -42,7 +42,7 @@ var person = new (Person)(); Examples of **incorrect** code for this rule with the `"never"` option: ```js -/*eslint new-parens: "error"*/ +/*eslint new-parens: ["error", "never"]*/ var person = new Person(); var person = new (Person)(); @@ -51,7 +51,7 @@ var person = new (Person)(); Examples of **correct** code for this rule with the `"never"` option: ```js -/*eslint new-parens: "error"*/ +/*eslint new-parens: ["error", "never"]*/ var person = new Person; var person = (new Person); From a4a910b2677952d6e35927003119a30876eb126e Mon Sep 17 00:00:00 2001 From: pfg Date: Fri, 29 Mar 2019 20:22:00 -0700 Subject: [PATCH 3/4] Chore: Add tests for new-parens explicit always and never with arguments --- tests/lib/rules/new-parens.js | 39 +++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/tests/lib/rules/new-parens.js b/tests/lib/rules/new-parens.js index 521f3d12619..4176abceaea 100644 --- a/tests/lib/rules/new-parens.js +++ b/tests/lib/rules/new-parens.js @@ -23,6 +23,8 @@ const ruleTester = new RuleTester(); ruleTester.run("new-parens", rule, { valid: [ + + // Default (Always) "var a = new Date();", "var a = new Date(function() {});", "var a = new (Date)();", @@ -30,7 +32,15 @@ ruleTester.run("new-parens", rule, { "var a = (new Date());", "var a = new foo.Bar();", "var a = (new Foo()).bar;", - { code: "new Storage('state');", parser: parser("typescript-parsers/new-parens") }, + { + code: "new Storage('state');", + parser: parser("typescript-parsers/new-parens") + }, + + // Explicit Always + { code: "var a = new Date();", options: ["always"] }, + { code: "var a = new foo.Bar();", options: ["always"] }, + { code: "var a = (new Foo()).bar;", options: ["always"] }, // Never { code: "var a = new Date;", options: ["never"] }, @@ -39,9 +49,14 @@ ruleTester.run("new-parens", rule, { { code: "var a = new ((Date));", options: ["never"] }, { code: "var a = (new Date);", options: ["never"] }, { code: "var a = new foo.Bar;", options: ["never"] }, - { code: "var a = (new Foo).bar;", options: ["never"] } + { code: "var a = (new Foo).bar;", options: ["never"] }, + { code: "var a = new Person('Name')", options: ["never"] }, + { code: "var a = new Person('Name', 12)", options: ["never"] }, + { code: "var a = new ((Person))('Name');", options: ["never"] } ], invalid: [ + + // Default (Always) { code: "var a = new Date;", output: "var a = new Date();", @@ -85,6 +100,26 @@ ruleTester.run("new-parens", rule, { errors: [error] }, + // Explicit always + { + code: "var a = new Date;", + output: "var a = new Date();", + options: ["always"], + errors: [error] + }, + { + code: "var a = new foo.Bar;", + output: "var a = new foo.Bar();", + options: ["always"], + errors: [error] + }, + { + code: "var a = (new Foo).bar;", + output: "var a = (new Foo()).bar;", + options: ["always"], + errors: [error] + }, + // Never { code: "var a = new Date();", From 2713dc0dd27f586c0853df860a30dd82e3b44296 Mon Sep 17 00:00:00 2001 From: pfg Date: Fri, 29 Mar 2019 20:25:40 -0700 Subject: [PATCH 4/4] Chore: Adjust wording and naming of new-parens unnecessary message --- lib/rules/new-parens.js | 8 ++++---- tests/lib/rules/new-parens.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/rules/new-parens.js b/lib/rules/new-parens.js index 60adc1eca40..6955284f1eb 100644 --- a/lib/rules/new-parens.js +++ b/lib/rules/new-parens.js @@ -24,7 +24,7 @@ module.exports = { type: "layout", docs: { - description: "require parentheses when invoking a constructor with no arguments", + description: "enforce or disallow parentheses when invoking a constructor with no arguments", category: "Stylistic Issues", recommended: false, url: "https://eslint.org/docs/rules/new-parens" @@ -47,13 +47,13 @@ module.exports = { }, messages: { missing: "Missing '()' invoking a constructor.", - using: "'()' included invoking a constructor with no arguments." + unnecessary: "Unnecessary '()' invoking a constructor with no arguments." } }, create(context) { const options = context.options; - const always = options[0] !== "never"; // Default is never + const always = options[0] !== "never"; // Default is always const sourceCode = context.getSourceCode(); @@ -79,7 +79,7 @@ module.exports = { if (hasParens) { context.report({ node, - messageId: "using", + messageId: "unnecessary", fix: fixer => [ fixer.remove(sourceCode.getTokenBefore(lastToken)), fixer.remove(lastToken), diff --git a/tests/lib/rules/new-parens.js b/tests/lib/rules/new-parens.js index 4176abceaea..b8bbba55c52 100644 --- a/tests/lib/rules/new-parens.js +++ b/tests/lib/rules/new-parens.js @@ -17,7 +17,7 @@ const parser = require("../../fixtures/fixture-parser"), // Tests //------------------------------------------------------------------------------ const error = { messageId: "missing", type: "NewExpression" }; -const neverError = { messageId: "using", type: "NewExpression" }; +const neverError = { messageId: "unnecessary", type: "NewExpression" }; const ruleTester = new RuleTester();