Skip to content

Commit cdc8e8c

Browse files
authored
fix: revert directive detection in no-unused-expressions (#19639)
* revert: use node.directive for directive check * format * add test * review
1 parent e62e267 commit cdc8e8c

File tree

3 files changed

+65
-3
lines changed

3 files changed

+65
-3
lines changed

lib/rules/no-unused-expressions.js

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ function alwaysFalse() {
2929
/** @type {import('../types').Rule.RuleModule} */
3030
module.exports = {
3131
meta: {
32+
dialects: ["javascript", "typescript"],
33+
language: "javascript",
3234
type: "suggestion",
3335

3436
docs: {
@@ -83,6 +85,61 @@ module.exports = {
8385
},
8486
] = context.options;
8587

88+
/**
89+
* Has AST suggesting a directive.
90+
* @param {ASTNode} node any node
91+
* @returns {boolean} whether the given node structurally represents a directive
92+
*/
93+
function looksLikeDirective(node) {
94+
return (
95+
node.type === "ExpressionStatement" &&
96+
node.expression.type === "Literal" &&
97+
typeof node.expression.value === "string"
98+
);
99+
}
100+
101+
/**
102+
* Gets the leading sequence of members in a list that pass the predicate.
103+
* @param {Function} predicate ([a] -> Boolean) the function used to make the determination
104+
* @param {a[]} list the input list
105+
* @returns {a[]} the leading sequence of members in the given list that pass the given predicate
106+
*/
107+
function takeWhile(predicate, list) {
108+
for (let i = 0; i < list.length; ++i) {
109+
if (!predicate(list[i])) {
110+
return list.slice(0, i);
111+
}
112+
}
113+
return list.slice();
114+
}
115+
116+
/**
117+
* Gets leading directives nodes in a Node body.
118+
* @param {ASTNode} node a Program or BlockStatement node
119+
* @returns {ASTNode[]} the leading sequence of directive nodes in the given node's body
120+
*/
121+
function directives(node) {
122+
return takeWhile(looksLikeDirective, node.body);
123+
}
124+
125+
/**
126+
* Detect if a Node is a directive.
127+
* @param {ASTNode} node any node
128+
* @returns {boolean} whether the given node is considered a directive in its current position
129+
*/
130+
function isDirective(node) {
131+
/**
132+
* https://tc39.es/ecma262/#directive-prologue
133+
*
134+
* Only `FunctionBody`, `ScriptBody` and `ModuleBody` can have directive prologue.
135+
* Class static blocks do not have directive prologue.
136+
*/
137+
return (
138+
astUtils.isTopLevelExpressionStatement(node) &&
139+
directives(node.parent).includes(node)
140+
);
141+
}
142+
86143
/**
87144
* The member functions return `true` if the type has no side-effects.
88145
* Unknown nodes are handled as `false`, then this rule ignores those.
@@ -154,7 +211,7 @@ module.exports = {
154211
ExpressionStatement(node) {
155212
if (
156213
Checker.isDisallowed(node.expression) &&
157-
!astUtils.isDirective(node)
214+
!isDirective(node)
158215
) {
159216
context.report({ node, messageId: "unusedExpression" });
160217
}

lib/rules/utils/ast-utils.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,13 +1088,13 @@ function isConstant(scope, node, inBooleanPosition) {
10881088
}
10891089

10901090
/**
1091-
* Checks whether a node is an ExpressionStatement at the top level of a file or function body.
1091+
* Checks whether a node is an ExpressionStatement at the top level of a file, function body, or TypeScript module block.
10921092
* A top-level ExpressionStatement node is a directive if it contains a single unparenthesized
10931093
* string literal and if it occurs either as the first sibling or immediately after another
10941094
* directive.
10951095
* @param {ASTNode} node The node to check.
10961096
* @returns {boolean} Whether or not the node is an ExpressionStatement at the top level of a
1097-
* file or function body.
1097+
* file, function body, or TypeScript module block.
10981098
*/
10991099
function isTopLevelExpressionStatement(node) {
11001100
if (node.type !== "ExpressionStatement") {
@@ -1104,6 +1104,7 @@ function isTopLevelExpressionStatement(node) {
11041104

11051105
return (
11061106
parent.type === "Program" ||
1107+
parent.type === "TSModuleBlock" ||
11071108
(parent.type === "BlockStatement" && isFunction(parent.parent))
11081109
);
11091110
}

tests/lib/rules/no-unused-expressions.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ ruleTester.run("no-unused-expressions", rule, {
3737
"delete foo.bar",
3838
"void new C",
3939
'"use strict";',
40+
{
41+
code: '"use strict";',
42+
languageOptions: { ecmaVersion: 3, sourceType: "script" },
43+
},
4044
'"directive one"; "directive two"; f();',
4145
'function foo() {"use strict"; return true; }',
4246
{

0 commit comments

Comments
 (0)