enhance keep_fargs (#3409)

LICENSE

@@ -1,6 +1,6 @@
 UglifyJS is released under the BSD license:
 
-Copyright 2012-2018 (c) Mihai Bazon <mihai.bazon@gmail.com>
+Copyright 2012-2019 (c) Mihai Bazon <mihai.bazon@gmail.com>
 
 Redistribution and use in source and binary forms, with or without
 modification, are permitted provided that the following conditions

README.md

@@ -664,8 +664,9 @@ If you're using the `X-SourceMap` header instead, you can just omit `sourceMap.u
 
 - `join_vars` (default: `true`) -- join consecutive `var` statements
 
-- `keep_fargs` (default: `true`) -- Prevents the compressor from discarding unused
-  function arguments.  You need this for code which relies on `Function.length`.
+- `keep_fargs` (default: `strict`) -- Discard unused function arguments. Code
+  which relies on `Function.length` will break if this is done indiscriminately,
+  i.e. when passing `true`. Pass `false` to always retain function arguments.
 
 - `keep_fnames` (default: `false`) -- Pass `true` to prevent the
   compressor from discarding function names.  Useful for code relying on

lib/ast.js

@@ -376,7 +376,7 @@ var AST_Toplevel = DEFNODE("Toplevel", "globals", {
     }
 }, AST_Scope);
 
-var AST_Lambda = DEFNODE("Lambda", "name argnames uses_arguments", {
+var AST_Lambda = DEFNODE("Lambda", "name argnames uses_arguments length_read", {
     $documentation: "Base class for functions",
     $propdoc: {
         name: "[AST_SymbolDeclaration?] the name of this function",
@@ -614,6 +614,18 @@ var AST_PropAccess = DEFNODE("PropAccess", "expression property", {
     $propdoc: {
         expression: "[AST_Node] the “container” expression",
         property: "[AST_Node|string] the property to access.  For AST_Dot this is always a plain string, while for AST_Sub it's an arbitrary AST_Node"
+    },
+    getProperty: function() {
+        var p = this.property;
+        if (p instanceof AST_Constant) {
+            return p.getValue();
+        }
+        if (p instanceof AST_UnaryPrefix
+            && p.operator == "void"
+            && p.expression instanceof AST_Constant) {
+            return;
+        }
+        return p;
     }
 });
 

lib/compress.js

@@ -69,7 +69,7 @@ function Compressor(options, false_by_default) {
         if_return       : !false_by_default,
         inline          : !false_by_default,
         join_vars       : !false_by_default,
-        keep_fargs      : true,
+        keep_fargs      : false_by_default || "strict",
         keep_fnames     : false,
         keep_infinity   : false,
         loops           : !false_by_default,
@@ -104,6 +104,17 @@ function Compressor(options, false_by_default) {
         }
     }
     if (this.options["inline"] === true) this.options["inline"] = 3;
+    var keep_fargs = this.options["keep_fargs"];
+    this.drop_fargs = keep_fargs == "strict" ? function(lambda) {
+        if (lambda.length_read) return false;
+        var name = lambda.name;
+        if (!name) return true;
+        if (name.fixed_value() !== lambda) return false;
+        var def = name.definition();
+        if (def.direct_access) return false;
+        var escaped = def.escaped;
+        return escaped && escaped.depth != 1;
+    } : keep_fargs ? return_false : return_true;
     var pure_funcs = this.options["pure_funcs"];
     if (typeof pure_funcs == "function") {
         this.pure_funcs = pure_funcs;
@@ -118,6 +129,8 @@ function Compressor(options, false_by_default) {
     } else {
         this.pure_funcs = return_true;
     }
+    var sequences = this.options["sequences"];
+    this.sequences_limit = sequences == 1 ? 800 : sequences | 0;
     var top_retain = this.options["top_retain"];
     if (top_retain instanceof RegExp) {
         this.top_retain = function(def) {
@@ -141,8 +154,6 @@ function Compressor(options, false_by_default) {
         funcs: toplevel,
         vars: toplevel
     };
-    var sequences = this.options["sequences"];
-    this.sequences_limit = sequences == 1 ? 800 : sequences | 0;
 }
 
 Compressor.prototype = new TreeTransformer;
@@ -272,14 +283,19 @@ merge(Compressor.prototype, {
         self.transform(tt);
     });
 
-    function read_property(obj, key) {
-        key = get_value(key);
+    function read_property(obj, node) {
+        var key = node.getProperty();
         if (key instanceof AST_Node) return;
         var value;
         if (obj instanceof AST_Array) {
             var elements = obj.elements;
             if (key == "length") return make_node_from_constant(elements.length, obj);
             if (typeof key == "number" && key in elements) value = elements[key];
+        } else if (obj instanceof AST_Lambda) {
+            if (key == "length") {
+                obj.length_read = true;
+                return make_node_from_constant(obj.argnames.length, obj);
+            }
         } else if (obj instanceof AST_Object) {
             key = "" + key;
             var props = obj.properties;
@@ -326,7 +342,7 @@ merge(Compressor.prototype, {
             return is_modified(compressor, tw, obj, obj, level + 2);
         }
         if (parent instanceof AST_PropAccess && parent.expression === node) {
-            var prop = read_property(value, parent.property);
+            var prop = read_property(value, parent);
             return !immutable && is_modified(compressor, tw, parent, prop, level + 1);
         }
     }
@@ -490,13 +506,15 @@ merge(Compressor.prototype, {
                 var obj = tw.parent(level + 1);
                 mark_escaped(tw, d, scope, obj, obj, level + 2, depth);
             } else if (parent instanceof AST_PropAccess && node === parent.expression) {
-                value = read_property(value, parent.property);
+                value = read_property(value, parent);
                 mark_escaped(tw, d, scope, parent, value, level + 1, depth + 1);
                 if (value) return;
             }
             if (level > 0) return;
+            if (parent instanceof AST_Call && node === parent.expression) return;
             if (parent instanceof AST_Sequence && node !== parent.tail_node()) return;
             if (parent instanceof AST_SimpleStatement) return;
+            if (parent instanceof AST_Unary) return;
             d.direct_access = true;
         }
 
@@ -2217,18 +2235,6 @@ merge(Compressor.prototype, {
         }));
     }
 
-    function get_value(key) {
-        if (key instanceof AST_Constant) {
-            return key.getValue();
-        }
-        if (key instanceof AST_UnaryPrefix
-            && key.operator == "void"
-            && key.expression instanceof AST_Constant) {
-            return;
-        }
-        return key;
-    }
-
     function is_undefined(node, compressor) {
         return node.is_undefined
             || node instanceof AST_Undefined
@@ -3621,7 +3627,7 @@ merge(Compressor.prototype, {
                 node.name = null;
             }
             if (node instanceof AST_Lambda && !(node instanceof AST_Accessor)) {
-                var trim = !compressor.option("keep_fargs");
+                var trim = compressor.drop_fargs(node);
                 for (var a = node.argnames, i = a.length; --i >= 0;) {
                     var sym = a[i];
                     if (!(sym.definition().id in in_use_ids)) {
@@ -4068,7 +4074,7 @@ merge(Compressor.prototype, {
             if (node instanceof AST_PropAccess && node.expression instanceof AST_SymbolRef) {
                 var defs = defs_by_id[node.expression.definition().id];
                 if (defs) {
-                    var def = defs.get(get_value(node.property));
+                    var def = defs.get(node.getProperty());
                     var sym = make_node(AST_SymbolRef, node, {
                         name: def.name,
                         scope: node.expression.scope,
@@ -6652,7 +6658,7 @@ merge(Compressor.prototype, {
                     || def.orig.length > 1) {
                     argname = null;
                 }
-            } else if (!argname && !compressor.option("keep_fargs") && index < fn.argnames.length + 5) {
+            } else if (!argname && compressor.drop_fargs(fn) && index < fn.argnames.length + 5) {
                 while (index >= fn.argnames.length) {
                     argname = make_node(AST_SymbolFunarg, fn, {
                         name: fn.make_var_name("argument_" + fn.argnames.length),
@@ -6665,6 +6671,7 @@ merge(Compressor.prototype, {
             if (argname && find_if(function(node) {
                 return node.name === argname.name;
             }, fn.argnames) === argname) {
+                expr.definition().reassigned = false;
                 var sym = make_node(AST_SymbolRef, self, argname);
                 sym.reference({});
                 delete argname.__unused;

test/compress/arguments.js

@@ -405,6 +405,52 @@ issue_3273_global_strict_reduce_vars: {
     ]
 }
 
+issue_3273_keep_fargs_false: {
+    options = {
+        arguments: true,
+        keep_fargs: false,
+        reduce_vars: true,
+    }
+    input: {
+        (function() {
+            "use strict";
+            arguments[0]++;
+            console.log(arguments[0]);
+        })(0);
+    }
+    expect: {
+        (function(argument_0) {
+            "use strict";
+            argument_0++;
+            console.log(argument_0);
+        })(0);
+    }
+    expect_stdout: "1"
+}
+
+issue_3273_keep_fargs_strict: {
+    options = {
+        arguments: true,
+        keep_fargs: "strict",
+        reduce_vars: true,
+    }
+    input: {
+        (function() {
+            "use strict";
+            arguments[0]++;
+            console.log(arguments[0]);
+        })(0);
+    }
+    expect: {
+        (function(argument_0) {
+            "use strict";
+            argument_0++;
+            console.log(argument_0);
+        })(0);
+    }
+    expect_stdout: "1"
+}
+
 issue_3282_1: {
     options = {
         arguments: true,