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.

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

feat: directive prologue detection and autofix condition in quotes #17284

Merged
merged 3 commits into from Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
6 changes: 5 additions & 1 deletion docs/src/rules/quotes.md
Expand Up @@ -15,14 +15,16 @@ var single = 'single';
var backtick = `backtick`; // ES6 only
```

Each of these lines creates a string and, in some cases, can be used interchangeably. The choice of how to define strings in a codebase is a stylistic one outside of template literals (which allow embedded of expressions to be interpreted).
Each of these lines creates a string and, in some cases, can be used interchangeably. The choice of how to define strings in a codebase is a stylistic one outside of template literals (which allow embedded expressions to be interpreted).

Many codebases require strings to be defined in a consistent manner.

## Rule Details

This rule enforces the consistent use of either backticks, double, or single quotes.

This rule is aware of directive prologues such as `"use strict";` and will not flag or autofix them unless doing so will not change how the directive prologue is interpreted.
Copy link
Member

Choose a reason for hiding this comment

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

A little difficult to parse, maybe:

Suggested change
This rule is aware of directive prologues such as `"use strict";` and will not flag or autofix them unless doing so will not change how the directive prologue is interpreted.
This rule is aware of directive prologues such as `"use strict"` and will not flag or autofix them if doing so will change how the directive prologue is interpreted.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I thought that the semicolon in "use strict"; would make it clearer that what is shown is a directive as opposed to just a string, but it that's bad for readability we can leave it out.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, dangling semicolons are a little visually confusing. :)


## Options

This rule has two options, a string option and an object option.
Expand Down Expand Up @@ -125,7 +127,9 @@ Examples of **correct** code for this rule with the `"backtick"` option:
/*eslint quotes: ["error", "backtick"]*/
/*eslint-env es6*/

"use strict"; // directives must use single or double quotes
var backtick = `backtick`;
var obj = { 'prop-name': `value` }; // backticks not allowed for property names
```

:::
Expand Down
28 changes: 14 additions & 14 deletions lib/rules/quotes.js
Expand Up @@ -157,7 +157,8 @@ module.exports = {

/**
* Checks whether or not a given node is a directive.
* The directive is a `ExpressionStatement` which has only a string literal.
* The directive is a `ExpressionStatement` which has only a string literal not surrounded by
* parentheses.
* @param {ASTNode} node A node to check.
* @returns {boolean} Whether or not the node is a directive.
* @private
Expand All @@ -166,23 +167,23 @@ module.exports = {
return (
node.type === "ExpressionStatement" &&
node.expression.type === "Literal" &&
typeof node.expression.value === "string"
typeof node.expression.value === "string" &&
!astUtils.isParenthesised(sourceCode, node.expression)
);
}

/**
* Checks whether or not a given node is a part of directive prologues.
* See also: http://www.ecma-international.org/ecma-262/6.0/#sec-directive-prologues-and-the-use-strict-directive
* Checks whether a specified node is either part of, or immediately follows a (possibly empty) directive prologue.
* @see {@link http://www.ecma-international.org/ecma-262/6.0/#sec-directive-prologues-and-the-use-strict-directive}
* @param {ASTNode} node A node to check.
* @returns {boolean} Whether or not the node is a part of directive prologues.
* @returns {boolean} Whether a specified node is either part of, or immediately follows a (possibly empty) directive prologue.
* @private
*/
function isPartOfDirectivePrologue(node) {
const block = node.parent.parent;

if (block.type !== "Program" && (block.type !== "BlockStatement" || !astUtils.isFunction(block.parent))) {
function isExpressionInOrJustAfterDirectivePrologue(node) {
if (!astUtils.isTopLevelExpressionStatement(node.parent)) {
return false;
}
const block = node.parent.parent;

// Check the node is at a prologue.
for (let i = 0; i < block.body.length; ++i) {
Expand Down Expand Up @@ -212,7 +213,7 @@ module.exports = {

// Directive Prologues.
case "ExpressionStatement":
return isPartOfDirectivePrologue(node);
return !astUtils.isParenthesised(sourceCode, node) && isExpressionInOrJustAfterDirectivePrologue(node);

// LiteralPropertyName.
case "Property":
Expand Down Expand Up @@ -328,12 +329,11 @@ module.exports = {
description: settings.description
},
fix(fixer) {
if (isPartOfDirectivePrologue(node)) {
if (astUtils.isTopLevelExpressionStatement(node.parent) && !astUtils.isParenthesised(sourceCode, node)) {

/*
* TemplateLiterals in a directive prologue aren't actually directives, but if they're
* in the directive prologue, then fixing them might turn them into directives and change
* the behavior of the code.
* TemplateLiterals aren't actually directives, but fixing them might turn
* them into directives and change the behavior of the code.
*/
return null;
}
Expand Down
60 changes: 58 additions & 2 deletions tests/lib/rules/quotes.js
Expand Up @@ -440,7 +440,7 @@ ruleTester.run("quotes", rule, {
},
{
code: "() => { foo(); `use strict`; }",
output: "() => { foo(); \"use strict\"; }",
output: null, // no autofix
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "wrongQuotes",
Expand All @@ -450,7 +450,7 @@ ruleTester.run("quotes", rule, {
},
{
code: "foo(); `use strict`;",
output: "foo(); \"use strict\";",
output: null, // no autofix
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "wrongQuotes",
Expand Down Expand Up @@ -725,6 +725,62 @@ ruleTester.run("quotes", rule, {
type: "Literal"
}
]
},

// https://github.com/eslint/eslint/pull/17022
{
code: "() => { foo(); (`use strict`); }",
output: "() => { foo(); (\"use strict\"); }",
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "wrongQuotes",
data: { description: "doublequote" },
type: "TemplateLiteral"
}]
},
{
code: "('foo'); \"bar\";",
output: "(`foo`); `bar`;",
options: ["backtick"],
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "wrongQuotes",
data: { description: "backtick" },
type: "Literal"
}, {
messageId: "wrongQuotes",
data: { description: "backtick" },
type: "Literal"
}]
},
{
code: "; 'use asm';",
output: "; \"use asm\";",
errors: [{
messageId: "wrongQuotes",
data: { description: "doublequote" },
type: "Literal"
}]
},
{
code: "{ `foobar`; }",
output: "{ \"foobar\"; }",
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "wrongQuotes",
data: { description: "doublequote" },
type: "TemplateLiteral"
}]
},
{
code: "foo(() => `bar`);",
output: "foo(() => \"bar\");",
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "wrongQuotes",
data: { description: "doublequote" },
type: "TemplateLiteral"
}]
}
]
});