Merge pull request #10262 from webpack/bugfix/pure-annotation-calls

the pure annotation only applies to function calls

lib/optimize/InnerGraphPlugin.js

@@ -21,23 +21,71 @@ const topLevelSymbolTag = Symbol("top level symbol");
 /**
  * @param {any} expr an expression
  * @param {JavascriptParser} parser the parser
+ * @param {number} commentsStartPos source position from which annotation comments are checked
  * @returns {boolean} true, when the expression is pure
  */
-const isPure = (expr, parser) => {
+const isPure = (expr, parser, commentsStartPos) => {
 	switch (expr.type) {
 		case "Identifier":
 			return (
 				parser.isVariableDefined(expr.name) ||
 				parser.getTagData(expr.name, harmonySpecifierTag)
 			);
+
+		case "ClassExpression":
+			if (expr.body.type !== "ClassBody") return false;
+			if (expr.superClass && !isPure(expr.superClass, parser, expr.range[0])) {
+				return false;
+			}
+			return expr.body.body.every(item => {
+				switch (item.type) {
+					case "ClassProperty":
+						// TODO add test case once acorn supports it
+						// Currently this is not parsable
+						if (item.static) return isPure(item.value, parser, item.range[0]);
+						break;
+				}
+				return true;
+			});
+
+		case "FunctionExpression":
+		case "ArrowFunctionExpression":
 		case "Literal":
 			return true;
+
 		case "ConditionalExpression":
 			return (
-				isPure(expr.test, parser) &&
-				isPure(expr.consequent, parser) &&
-				isPure(expr.alternate, parser)
+				isPure(expr.test, parser, commentsStartPos) &&
+				isPure(expr.consequent, parser, expr.test.range[1]) &&
+				isPure(expr.alternate, parser, expr.consequent.range[1])
 			);
+
+		case "SequenceExpression":
+			return expr.expressions.every(expr => {
+				const pureFlag = isPure(expr, parser, commentsStartPos);
+				commentsStartPos = expr.range[1];
+				return pureFlag;
+			});
+
+		case "CallExpression": {
+			const pureFlag =
+				expr.range[0] - commentsStartPos > 12 &&
+				parser
+					.getComments([commentsStartPos, expr.range[0]])
+					.some(
+						comment =>
+							comment.type === "Block" &&
+							/^\s*(#|@)__PURE__\s*$/.test(comment.value)
+					);
+			if (!pureFlag) return false;
+			commentsStartPos = expr.callee.range[1];
+			return expr.arguments.every(arg => {
+				if (arg.type === "SpreadElement") return false;
+				const pureFlag = isPure(arg, parser, commentsStartPos);
+				commentsStartPos = arg.range[1];
+				return pureFlag;
+			});
+		}
 	}
 	return false;
 };
@@ -235,17 +283,7 @@ class InnerGraphPlugin {
 									declWithTopLevelSymbol.set(decl, fn);
 									return true;
 								}
-								if (
-									(decl.init.range[0] - decl.id.range[1] > 9 &&
-										parser
-											.getComments([decl.id.range[1], decl.init.range[0]])
-											.some(
-												comment =>
-													comment.type === "Block" &&
-													/^\s*(#|@)__PURE__\s*$/.test(comment.value)
-											)) ||
-									isPure(decl.init, parser)
-								) {
+								if (isPure(decl.init, parser, decl.id.range[1])) {
 									const name = decl.id.name;
 									const fn = tagVar(name);
 									declWithTopLevelSymbol.set(decl, fn);

test/cases/inner-graph/simple/index.js

@@ -1,4 +1,11 @@
-import { exportUsed, export2Used, export3Used, export4Used } from "./inner";
+import {
+	exportUsed,
+	export2Used,
+	export3Used,
+	export4Used,
+	export5Used,
+	export6Used
+} from "./inner";
 import { f1, pureUsed, fWithDefault } from "./module";
 
 it("export should be unused when only unused functions use it", () => {
@@ -10,6 +17,8 @@ it("export should be unused when only unused functions use it", () => {
 		expect(export2Used).toBe(true);
 		expect(export3Used).toBe(true);
 		expect(export4Used).toBe(true);
+		expect(export5Used).toBe(true);
+		expect(export6Used).toBe(true);
 	}
 	return import("./chunk");
 });

test/cases/inner-graph/simple/inner.js

@@ -2,8 +2,12 @@ export const EXPORT = 42;
 export const EXPORT2 = 42;
 export const EXPORT3 = 42;
 export const EXPORT4 = 42;
+export const EXPORT5 = () => 42;
+export const EXPORT6 = () => 42;
 
 export const exportUsed = __webpack_exports_info__.EXPORT.used;
 export const export2Used = __webpack_exports_info__.EXPORT2.used;
 export const export3Used = __webpack_exports_info__.EXPORT3.used;
 export const export4Used = __webpack_exports_info__.EXPORT4.used;
+export const export5Used = __webpack_exports_info__.EXPORT5.used;
+export const export6Used = __webpack_exports_info__.EXPORT6.used;

test/cases/inner-graph/simple/module.js

@@ -1,4 +1,4 @@
-import { EXPORT, EXPORT2, EXPORT3, EXPORT4 } from "./inner";
+import { EXPORT, EXPORT2, EXPORT3, EXPORT4, EXPORT5, EXPORT6 } from "./inner";
 
 export function f1() {
 	// no using EXPORT
@@ -24,6 +24,14 @@ let f6 = () => {
 	return EXPORT;
 };
 
+const f7 = () => {
+	return EXPORT5();
+};
+
+const f8 = () => {
+	return EXPORT6();
+};
+
 export function g2() {
 	return f2();
 }
@@ -52,10 +60,22 @@ export class g7 {
 	}
 }
 
-export const pure1 = /*#__PURE__*/ EXPORT;
+export const pure1 = EXPORT;
 export const pure2 = /*#__PURE__*/ f6();
 const pure3 = /*#__PURE__*/ g5();
-export const pureUsed = /*#__PURE__*/ EXPORT3;
+const pure4 = /*#__PURE__*/ f7(f8());
+const pure5 =
+	("fefef", 1123, /*#__PURE__*/ f2("fwefe"), /*#__PURE__*/ f2("efwefa"));
+const pure6 = /*#__PURE__*/ f2(/*#__PURE__*/ f2(), /*#__PURE__*/ f2());
+const pure7 = /*#__PURE__*/ f2(
+	class {
+		f() {
+			return EXPORT;
+		}
+	}
+);
+const pure8 = /*#__PURE__*/ f2(() => EXPORT);
+export const pureUsed = EXPORT3;
 
 function x1() {
 	return EXPORT2;