feat: add dead control flow check

This commit is contained in:
Ivan Kopeykin 2025-02-28 17:39:34 +03:00 committed by GitHub
parent 0377fb1643
commit af7d788437
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 467 additions and 25 deletions

View File

@ -22,6 +22,7 @@ const { parseResource } = require("./util/identifier");
/** @typedef {import("estree").SourceLocation} SourceLocation */
/** @typedef {import("estree").Statement} Statement */
/** @typedef {import("estree").Super} Super */
/** @typedef {import("estree").VariableDeclaration} VariableDeclaration */
/** @typedef {import("./Compiler")} Compiler */
/** @typedef {import("./javascript/BasicEvaluatedExpression")} BasicEvaluatedExpression */
/** @typedef {import("./javascript/JavascriptParser")} JavascriptParser */
@ -68,7 +69,7 @@ const collectDeclaration = (declarations, pattern) => {
*/
const getHoistedDeclarations = (branch, includeFunctionDeclarations) => {
const declarations = new Set();
/** @type {Array<TODO | null | undefined>} */
/** @type {Array<Statement | null | undefined>} */
const stack = [branch];
while (stack.length > 0) {
const node = stack.pop();
@ -87,12 +88,12 @@ const getHoistedDeclarations = (branch, includeFunctionDeclarations) => {
stack.push(node.alternate);
break;
case "ForStatement":
stack.push(node.init);
stack.push(/** @type {VariableDeclaration} */ (node.init));
stack.push(node.body);
break;
case "ForInStatement":
case "ForOfStatement":
stack.push(node.left);
stack.push(/** @type {VariableDeclaration} */ (node.left));
stack.push(node.body);
break;
case "DoWhileStatement":
@ -158,6 +159,7 @@ class ConstPlugin {
* @param {JavascriptParser} parser the parser
*/
const handler = parser => {
parser.hooks.terminate.tap(PLUGIN_NAME, statement => true);
parser.hooks.statementIf.tap(PLUGIN_NAME, statement => {
if (parser.scope.isAsmJs) return;
const param = parser.evaluateExpression(statement.test);

View File

@ -124,6 +124,7 @@ const importAssertions = Parser =>
/** @type {unknown} */ (
class extends Parser {
parseWithClause() {
/** @type {ImportAttribute[]} */
const nodes = [];
const isAssertLegacy = this.value === "assert";
@ -287,6 +288,8 @@ class VariableInfo {
* @property {boolean} inTry
* @property {boolean} isStrict
* @property {boolean} isAsmJs
* @property {boolean} inExecutedPath false for unknown state
* @property {undefined|"return"|"throw"} terminated
*/
/** @typedef {[number, number]} Range */
@ -563,6 +566,8 @@ class JavascriptParser extends Parser {
expressionLogicalOperator: new SyncBailHook(["expression"]),
/** @type {SyncBailHook<[Program, Comment[]], boolean | void>} */
program: new SyncBailHook(["ast", "comments"]),
/** @type {SyncBailHook<[ThrowStatement | ReturnStatement], boolean | void>} */
terminate: new SyncBailHook(["statement"]),
/** @type {SyncBailHook<[Program, Comment[]], boolean | void>} */
finish: new SyncBailHook(["ast", "comments"])
});
@ -1941,6 +1946,7 @@ class JavascriptParser extends Parser {
for (let index = 0, len = statements.length; index < len; index++) {
const statement = statements[index];
this.walkStatement(statement);
if (this.scope.terminated) break;
}
}
@ -2147,7 +2153,7 @@ class JavascriptParser extends Parser {
this.blockPreWalkStatements(body);
this.prevStatement = prev;
this.walkStatements(body);
});
}, this.scope.inExecutedPath);
}
/**
@ -2173,15 +2179,21 @@ class JavascriptParser extends Parser {
walkIfStatement(statement) {
const result = this.hooks.statementIf.call(statement);
if (result === undefined) {
this.walkExpression(statement.test);
this.walkNestedStatement(statement.consequent);
if (statement.alternate) {
this.walkNestedStatement(statement.alternate);
}
} else if (result) {
this.walkNestedStatement(statement.consequent);
} else if (statement.alternate) {
this.walkNestedStatement(statement.alternate);
this.inExecutedPath(false, () => {
this.walkExpression(statement.test);
this.walkNestedStatement(statement.consequent);
if (statement.alternate) {
this.walkNestedStatement(statement.alternate);
}
});
} else {
this.inExecutedPath(true, () => {
if (result) {
this.walkNestedStatement(statement.consequent);
} else if (statement.alternate) {
this.walkNestedStatement(statement.alternate);
}
});
}
}
@ -2239,6 +2251,12 @@ class JavascriptParser extends Parser {
*/
walkTerminatingStatement(statement) {
if (statement.argument) this.walkExpression(statement.argument);
// Skip top level scope because to handle `export` and `module.exports` after terminate
if (this.scope.topLevelScope === true) return;
if (this.hooks.terminate.call(statement)) {
this.scope.terminated =
statement.type === "ReturnStatement" ? "return" : "throw";
}
}
/**
@ -2943,8 +2961,10 @@ class JavascriptParser extends Parser {
if (switchCase.test) {
this.walkExpression(switchCase.test);
}
if (switchCase.consequent.length > 0) {
this.walkStatements(switchCase.consequent);
this.scope.terminated = undefined;
}
}
});
@ -3422,15 +3442,21 @@ class JavascriptParser extends Parser {
walkConditionalExpression(expression) {
const result = this.hooks.expressionConditionalOperator.call(expression);
if (result === undefined) {
this.walkExpression(expression.test);
this.walkExpression(expression.consequent);
if (expression.alternate) {
this.walkExpression(expression.alternate);
}
} else if (result) {
this.walkExpression(expression.consequent);
} else if (expression.alternate) {
this.walkExpression(expression.alternate);
this.inExecutedPath(false, () => {
this.walkExpression(expression.test);
this.walkExpression(expression.consequent);
if (expression.alternate) {
this.walkExpression(expression.alternate);
}
});
} else {
this.inExecutedPath(true, () => {
if (result) {
this.walkExpression(expression.consequent);
} else if (expression.alternate) {
this.walkExpression(expression.alternate);
}
});
}
}
@ -3997,6 +4023,8 @@ class JavascriptParser extends Parser {
inTaggedTemplateTag: false,
isStrict: oldScope.isStrict,
isAsmJs: oldScope.isAsmJs,
inExecutedPath: false,
terminated: undefined,
definitions: oldScope.definitions.createChild()
};
@ -4011,6 +4039,23 @@ class JavascriptParser extends Parser {
this.scope = oldScope;
}
/**
* @param {boolean} state executed state
* @param {function(): void} fn inner function
*/
inExecutedPath(state, fn) {
const oldState = this.scope.inExecutedPath;
const oldTerminated = this.scope.terminated;
this.scope.inExecutedPath = state;
fn();
if (!state) {
this.scope.terminated = oldTerminated;
}
this.scope.inExecutedPath = oldState;
}
/**
* @param {boolean} hasThis true, when this is defined
* @param {Identifier[]} params scope params
@ -4024,8 +4069,10 @@ class JavascriptParser extends Parser {
inTry: false,
inShorthand: false,
inTaggedTemplateTag: false,
inExecutedPath: true,
isStrict: oldScope.isStrict,
isAsmJs: oldScope.isAsmJs,
terminated: undefined,
definitions: oldScope.definitions.createChild()
};
@ -4055,8 +4102,10 @@ class JavascriptParser extends Parser {
inTry: false,
inShorthand: false,
inTaggedTemplateTag: false,
inExecutedPath: true,
isStrict: oldScope.isStrict,
isAsmJs: oldScope.isAsmJs,
terminated: undefined,
definitions: oldScope.definitions.createChild()
};
@ -4075,22 +4124,31 @@ class JavascriptParser extends Parser {
/**
* @param {function(): void} fn inner function
* @param {boolean} inExecutedPath executed state
* @returns {void}
*/
inBlockScope(fn) {
inBlockScope(fn, inExecutedPath = false) {
const oldScope = this.scope;
this.scope = {
topLevelScope: oldScope.topLevelScope,
inTry: oldScope.inTry,
inShorthand: false,
inTaggedTemplateTag: false,
inExecutedPath,
isStrict: oldScope.isStrict,
isAsmJs: oldScope.isAsmJs,
terminated: oldScope.terminated,
definitions: oldScope.definitions.createChild()
};
fn();
const terminated = this.scope.terminated;
if (inExecutedPath && (!this.scope.inTry || terminated !== "throw")) {
oldScope.terminated = terminated;
}
this.scope = oldScope;
}
@ -4406,8 +4464,10 @@ class JavascriptParser extends Parser {
inTry: false,
inShorthand: false,
inTaggedTemplateTag: false,
inExecutedPath: false,
isStrict: false,
isAsmJs: false,
terminated: undefined,
definitions: new StackedMap()
};
/** @type {ParserState} */

View File

@ -28,7 +28,7 @@ const tests = fs
const filterPath = path.join(testDirectory, "test.filter.js");
if (fs.existsSync(filterPath) && !require(filterPath)()) {
// eslint-disable-next-line jest/no-disabled-tests, jest/valid-describe-callback
describe.skip(testName, () => it("filtered"));
describe.skip(testName, () => it("filtered", () => {}));
return false;
}
return true;

View File

@ -4772,6 +4772,19 @@ global:
global (webpack x.x.x) compiled successfully in X ms"
`;
exports[`StatsTestCases should print correct stats for track-returned 1`] = `
"asset bundle.js X KiB [emitted] (name: main)
./index.js X KiB [built] [code generated]
./used3.js X bytes [built] [code generated]
./used.js X bytes [built] [code generated]
./used1.js X bytes [built] [code generated]
./used4.js X bytes [built] [code generated]
./used2.js X bytes [built] [code generated]
./used5.js X bytes [built] [code generated]
./used6.js X bytes [built] [code generated]
webpack x.x.x compiled successfully in X ms"
`;
exports[`StatsTestCases should print correct stats for tree-shaking 1`] = `
"asset bundle.js X KiB [emitted] (name: main)
runtime modules X bytes 3 modules

View File

@ -19,6 +19,16 @@ it("should allow to import an rejected async module again", async () => {
message: expect.stringContaining("expected rejection 1")
})
);
try {
require("./script")
} catch (e) {
expect.stringContaining("expected rejection 1")
}
try {
require("./script-reexport")
} catch (e) {
expect.stringContaining("expected rejection 1")
}
await Promise.all([
expect(require("./module?3")).rejects.toEqual(
expect.objectContaining({

View File

@ -0,0 +1 @@
module.exports = require("./script");

View File

@ -0,0 +1,4 @@
const c = 1;
throw new Error("expected rejection " + c);
module.exports = "ok";

View File

@ -0,0 +1,332 @@
function rand() {
return Math.random() > 0.5;
}
it("should track return in function declaration", () => {
function a1() {
return;
require("fail");
}
function a2() {
if (true) return;
require("fail");
}
function a3() {
{
{
if (true) return;
require("fail");
}
}
}
function a4() {
if (true) {
{
{}
return;
require("fail");
}
}
}
function a5() {
if (rand()) {
return;
throw require("fail");
}
if (rand()) return;
require("./used3");
}
a1();
a2();
a3();
a4();
a5();
});
it("should track return in function expression", () => {
const a1 = function () {
return;
require("fail");
}
const a2 = function () {
if (true) return;
require("fail");
}
const a3 = function () {
{
{
if (true) return;
require("fail");
}
}
}
const a4 = function () {
if (true) {
{
{}
return;
require("fail");
}
}
}
const a5 = function () {
if (rand()) {
return;
throw require("fail");
}
}
a1();
a2();
a3();
a4();
a5();
});
it("should track return in arrow function expression", () => {
const a1 = () => {
return;
require("fail");
}
const a2 = () => {
if (true) return;
result = require("fail");
}
const a3 = () => {
{
{
if (true) return;
result = require("fail");
}
}
}
const a4 = () => {
if (true) {
{
{}
return;
result = require("fail");
}
}
}
const a5 = () => {
if (rand()) {
return;
throw require("fail");
}
}
const a6 = () => {
if (true) {
return;
(() => require("fail"))()
}
}
a1();
a2();
a3();
a4();
a5();
a6();
});
it("should work correct for try catch and loops", () => {
try {
throw 1;
require("fail");
} catch (e) {
require('./used');
}
function test() {
try {
return;
require("fail");
} finally {
require('./used');
}
}
for(let i = 0; i < 1; i++)
if (rand())
require('./used1');
for(let i = 0; i < 1; i++) {
if (true) {
require('./used4');
return;
}
import("fail");
}
try {
if (rand()) {
if (true) return;
require("fail");
}
return;
} catch {}
require("fail");
});
it("should handle edge case with switch case", () => {
const a = rand() ? 1 : 2;
switch (a) {
case 1: {
if (true) return;
return require("fail");
}
case 2:
if (true) return;
return require("fail");
default:
require("./used2");
}
});
it("should work correct for if", () => {
if (true) {
require('./used');
return;
}
require("fail");
});
it("should work correct for if #2", () => {
if (false) {
require("fail");
} else {
require('./used');
}
});
it("should work correct for if #3", () => {
if (false) {
require("fail");
} else if (true) {
require('./used');
} else {
require("fail");
}
});
it("should work correct for if #4", () => {
if (false) {
require("fail");
} else if (false) {
require("fail");
} else {
require('./used');
}
});
it("should not include unused assets", (done) => {
let a, b;
(function() {
try {
return;
require("fail");
} finally {
a = require('./used')
{
try {
return;
require("fail");
} finally {
b = require('./used')
}
}
require("fail");
}
})();
});
it("should work correct for classes", () => {
class Test {
value = true ? require('./used') : require("fail");
static value = true ? require('./used') : require("fail");
constructor(height = true ? require('./used') : require("fail"), width) {
if (true) return;
return require("fail");
}
method() {
if (true) return;
return require("fail");
}
static method() {
if (true) return;
return require("fail");
}
get area() {
if (true) return;
return require("fail");
}
set area(value) {
if (true) return;
return require("fail");
}
}
});
function top1() {
return;
require("fail");
}
if (false) {
require("fail");
} else if (true) {
require('./used');
} else {
require("fail");
}
const test = true ? require('./used') : require("fail");
const a = rand() ? 1 : 2;
switch (a) {
case 1: {
if (true) return;
else require("fail");
}
case 2:
if (false) require("fail");
default:
require("./used2");
}
if (true) {
require("./used5");
}
if (false) {
require("fail");
}
require("./used6");

View File

@ -0,0 +1,5 @@
module.exports = {
validate(stats) {
expect(stats.compilation.modules.size).toBe(8);
}
};

View File

@ -0,0 +1 @@
module.exports = 10;

View File

@ -0,0 +1 @@
module.exports = 10;

View File

@ -0,0 +1 @@
module.exports = 10;

View File

@ -0,0 +1 @@
module.exports = 10;

View File

@ -0,0 +1 @@
module.exports = 10;

View File

@ -0,0 +1 @@
module.exports = 10;

View File

@ -0,0 +1 @@
module.exports = 10;

10
types.d.ts vendored
View File

@ -6307,6 +6307,7 @@ declare class JavascriptParser extends Parser {
boolean | void
>;
program: SyncBailHook<[Program, Comment[]], boolean | void>;
terminate: SyncBailHook<[ReturnStatement | ThrowStatement], boolean | void>;
finish: SyncBailHook<[Program, Comment[]], boolean | void>;
}>;
sourceType: "module" | "auto" | "script";
@ -6842,6 +6843,7 @@ declare class JavascriptParser extends Parser {
...args: AsArray<T>
): undefined | R;
inScope(params: any, fn: () => void): void;
inExecutedPath(state: boolean, fn: () => void): void;
inClassScope(hasThis: boolean, params: Identifier[], fn: () => void): void;
inFunctionScope(
hasThis: boolean,
@ -6856,7 +6858,7 @@ declare class JavascriptParser extends Parser {
)[],
fn: () => void
): void;
inBlockScope(fn: () => void): void;
inBlockScope(fn: () => void, inExecutedPath?: boolean): void;
detectMode(
statements: (
| ImportDeclarationJavascriptParser
@ -13927,6 +13929,12 @@ declare interface ScopeInfo {
inTry: boolean;
isStrict: boolean;
isAsmJs: boolean;
/**
* false for unknown state
*/
inExecutedPath: boolean;
terminated?: "return" | "throw";
}
declare interface Selector<A, B> {
(input: A): undefined | null | B;