diff --git a/docs/src/rules/no-extra-parens.md b/docs/src/rules/no-extra-parens.md index 9b1b8df0725..dd2b4642008 100644 --- a/docs/src/rules/no-extra-parens.md +++ b/docs/src/rules/no-extra-parens.md @@ -21,6 +21,25 @@ This rule always ignores extra parentheses around the following: * immediately-invoked function expressions (also known as IIFEs) such as `var x = (function () {})();` and `var x = (function () {}());` to avoid conflicts with the [wrap-iife](wrap-iife) rule * arrow function arguments to avoid conflicts with the [arrow-parens](arrow-parens) rule +Problems reported by this rule can be fixed automatically, except when removing the parentheses would create a new directive, because that could change the semantics of the code. +For example, the following script prints `object` to the console, but if the parentheses around `"use strict"` were removed, it would print `undefined` instead. + +```js + + +("use strict"); + +function test() { + console.log(typeof this); +} + +test(); +``` + +In this case, the rule will not try to remove the parentheses around `"use strict"` but will still report them as a problem. + ## Options This rule has a string option: diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js index 0f59d7dd382..e9080120fab 100644 --- a/lib/rules/no-extra-parens.js +++ b/lib/rules/no-extra-parens.js @@ -386,6 +386,30 @@ module.exports = { return node && (node.type === "Identifier" || node.type === "MemberExpression"); } + /** + * Checks if a node is fixable. + * A node is fixable if removing a single pair of surrounding parentheses does not turn it + * into a directive after fixing other nodes. + * Almost all nodes are fixable, except if all of the following conditions are met: + * The node is a string Literal + * It has a single pair of parentheses + * It is the only child of an ExpressionStatement + * @param {ASTNode} node The node to evaluate. + * @returns {boolean} Whether or not the node is fixable. + * @private + */ + function isFixable(node) { + + // if it's not a string literal it can be autofixed + if (node.type !== "Literal" || typeof node.value !== "string") { + return true; + } + if (isParenthesisedTwice(node)) { + return true; + } + return !astUtils.isTopLevelExpressionStatement(node.parent); + } + /** * Report the node * @param {ASTNode} node node to evaluate @@ -429,14 +453,16 @@ module.exports = { node, loc: leftParenToken.loc, messageId: "unexpected", - fix(fixer) { - const parenthesizedSource = sourceCode.text.slice(leftParenToken.range[1], rightParenToken.range[0]); - - return fixer.replaceTextRange([ - leftParenToken.range[0], - rightParenToken.range[1] - ], (requiresLeadingSpace(node) ? " " : "") + parenthesizedSource + (requiresTrailingSpace(node) ? " " : "")); - } + fix: isFixable(node) + ? fixer => { + const parenthesizedSource = sourceCode.text.slice(leftParenToken.range[1], rightParenToken.range[0]); + + return fixer.replaceTextRange([ + leftParenToken.range[0], + rightParenToken.range[1] + ], (requiresLeadingSpace(node) ? " " : "") + parenthesizedSource + (requiresTrailingSpace(node) ? " " : "")); + } + : null }); } diff --git a/lib/rules/no-unused-expressions.js b/lib/rules/no-unused-expressions.js index 8337c19487e..46bb7baac22 100644 --- a/lib/rules/no-unused-expressions.js +++ b/lib/rules/no-unused-expressions.js @@ -4,6 +4,8 @@ */ "use strict"; +const astUtils = require("./utils/ast-utils"); + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -112,8 +114,6 @@ module.exports = { * @returns {boolean} whether the given node is considered a directive in its current position */ function isDirective(node) { - const parent = node.parent, - grandparent = parent.parent; /** * https://tc39.es/ecma262/#directive-prologue @@ -121,9 +121,7 @@ module.exports = { * Only `FunctionBody`, `ScriptBody` and `ModuleBody` can have directive prologue. * Class static blocks do not have directive prologue. */ - return (parent.type === "Program" || parent.type === "BlockStatement" && - (/Function/u.test(grandparent.type))) && - directives(parent).includes(node); + return astUtils.isTopLevelExpressionStatement(node) && directives(node.parent).includes(node); } /** diff --git a/lib/rules/padding-line-between-statements.js b/lib/rules/padding-line-between-statements.js index be7e2e40488..6b165c07f27 100644 --- a/lib/rules/padding-line-between-statements.js +++ b/lib/rules/padding-line-between-statements.js @@ -138,14 +138,7 @@ function isBlockLikeStatement(sourceCode, node) { */ function isDirective(node, sourceCode) { return ( - node.type === "ExpressionStatement" && - ( - node.parent.type === "Program" || - ( - node.parent.type === "BlockStatement" && - astUtils.isFunction(node.parent.parent) - ) - ) && + astUtils.isTopLevelExpressionStatement(node) && node.expression.type === "Literal" && typeof node.expression.value === "string" && !astUtils.isParenthesised(sourceCode, node.expression) diff --git a/lib/rules/utils/ast-utils.js b/lib/rules/utils/ast-utils.js index f4a18cff783..aed2c42f9a3 100644 --- a/lib/rules/utils/ast-utils.js +++ b/lib/rules/utils/ast-utils.js @@ -986,6 +986,25 @@ function isConstant(scope, node, inBooleanPosition) { return false; } +/** + * Checks whether a node is an ExpressionStatement at the top level of a file or function body. + * A top-level ExpressionStatement node is a directive if it contains a single unparenthesized + * string literal and if it occurs either as the first sibling or immediately after another + * directive. + * @param {ASTNode} node The node to check. + * @returns {boolean} Whether or not the node is an ExpressionStatement at the top level of a + * file or function body. + */ +function isTopLevelExpressionStatement(node) { + if (node.type !== "ExpressionStatement") { + return false; + } + const parent = node.parent; + + return parent.type === "Program" || (parent.type === "BlockStatement" && isFunction(parent.parent)); + +} + //------------------------------------------------------------------------------ // Public Interface //------------------------------------------------------------------------------ @@ -1501,7 +1520,6 @@ module.exports = { return directives; }, - /** * Determines whether this node is a decimal integer literal. If a node is a decimal integer literal, a dot added * after the node will be parsed as a decimal point, rather than a property-access dot. @@ -2120,5 +2138,6 @@ module.exports = { isLogicalAssignmentOperator, getSwitchCaseColonToken, getModuleExportName, - isConstant + isConstant, + isTopLevelExpressionStatement }; diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js index c51a47cfd3f..71e1e2e9845 100644 --- a/tests/lib/rules/no-extra-parens.js +++ b/tests/lib/rules/no-extra-parens.js @@ -3444,6 +3444,24 @@ ruleTester.run("no-extra-parens", rule, { `a ${operator} function () {};`, "Identifier" ) - ) + ), + + // Potential directives (no autofix) + invalid("('use strict');", null), + invalid("function f() { ('abc'); }", null), + invalid("(function () { ('abc'); })();", null), + invalid("_ = () => { ('abc'); };", null), + invalid("'use strict';(\"foobar\");", null), + invalid("foo(); ('bar');", null), + invalid("foo = { bar() { ; (\"baz\"); } };", null), + + // Directive lookalikes + invalid("(12345);", "12345;"), + invalid("(('foobar'));", "('foobar');"), + invalid("(`foobar`);", "`foobar`;"), + invalid("void ('foobar');", "void 'foobar';"), + invalid("_ = () => ('abc');", "_ = () => 'abc';"), + invalid("if (foo) ('bar');", "if (foo) 'bar';"), + invalid("const foo = () => ('bar');", "const foo = () => 'bar';") ] }); diff --git a/tests/lib/rules/utils/ast-utils.js b/tests/lib/rules/utils/ast-utils.js index c2a9201ac73..b0b9cb57cdf 100644 --- a/tests/lib/rules/utils/ast-utils.js +++ b/tests/lib/rules/utils/ast-utils.js @@ -1802,4 +1802,59 @@ describe("ast-utils", () => { }); }); }); + + describe("isTopLevelExpressionStatement", () => { + it("should return false for a Program node", () => { + const node = { type: "Program", parent: null }; + + assert.strictEqual(astUtils.isTopLevelExpressionStatement(node), false); + }); + + it("should return false if the node is not an ExpressionStatement", () => { + linter.defineRule("checker", { + create: mustCall(() => ({ + ":expression": mustCall(node => { + assert.strictEqual(astUtils.isTopLevelExpressionStatement(node), false); + }) + })) + }); + + linter.verify("var foo = () => \"use strict\";", { rules: { checker: "error" }, parserOptions: { ecmaVersion: 2022 } }); + }); + + const expectedResults = [ + ["if (foo) { \"use strict\"; }", "\"use strict\";", false], + ["{ \"use strict\"; }", "\"use strict\";", false], + ["switch (foo) { case bar: \"use strict\"; }", "\"use strict\";", false], + ["foo; bar;", "foo;", true], + ["foo; bar;", "bar;", true], + ["function foo() { bar; }", "bar;", true], + ["var foo = function () { foo(); };", "foo();", true], + ["var foo = () => { 'bar'; }", "'bar';", true], + ["\"use strict\"", "\"use strict\"", true], + ["(`use strict`)", "(`use strict`)", true] + ]; + + expectedResults.forEach(([code, nodeText, expectedRetVal]) => { + it(`should return ${expectedRetVal} for \`${nodeText}\` in \`${code}\``, () => { + linter.defineRule("checker", { + create: mustCall(context => { + const assertForNode = mustCall( + node => assert.strictEqual(astUtils.isTopLevelExpressionStatement(node), expectedRetVal) + ); + + return ({ + ExpressionStatement(node) { + if (context.sourceCode.getText(node) === nodeText) { + assertForNode(node); + } + } + }); + }) + }); + + linter.verify(code, { rules: { checker: "error" }, parserOptions: { ecmaVersion: 2022 } }); + }); + }); + }); });