Bug 1337465 - Create proper help strings for JS shell namespace objects, r=jonco
authorSteve Fink <sfink@mozilla.com>
Fri, 17 Feb 2017 14:04:06 -0800
changeset 373855 1d4116cb3c9d281dfcb3c5c183dc6c612dbbd980
parent 373854 e3e72862322d8545ae733e793d7222c184e72961
child 373856 96b7a5189c502578e4ad45090b576e222e659785
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1337465
milestone54.0a1
Bug 1337465 - Create proper help strings for JS shell namespace objects, r=jonco MozReview-Commit-ID: Ha6gkvgUIgZ
js/src/shell/OSObject.cpp
js/src/shell/jsshell.cpp
js/src/shell/jsshell.h
--- a/js/src/shell/OSObject.cpp
+++ b/js/src/shell/OSObject.cpp
@@ -996,35 +996,40 @@ DefineOS(JSContext* cx, HandleObject glo
     if (!GenerateInterfaceHelp(cx, obj, "os"))
         return false;
 
     gOutFilePtr = shellOut;
     gErrFilePtr = shellErr;
 
     // For backwards compatibility, expose various os.file.* functions as
     // direct methods on the global.
-    RootedValue val(cx);
-
-    struct {
+    struct Export {
         const char* src;
         const char* dst;
-    } osfile_exports[] = {
+    };
+
+    const Export osfile_exports[] = {
         { "readFile", "read" },
         { "readFile", "snarf" },
         { "readRelativeToScript", "readRelativeToScript" },
-        { "redirect", "redirect" },
-        { "redirectErr", "redirectErr" }
     };
 
     for (auto pair : osfile_exports) {
-        if (!JS_GetProperty(cx, osfile, pair.src, &val))
+        if (!CreateAlias(cx, pair.dst, osfile, pair.src))
             return false;
-        if (val.isObject()) {
-            RootedObject function(cx, &val.toObject());
-            if (!JS_DefineProperty(cx, global, pair.dst, function, 0))
+    }
+
+    if (!fuzzingSafe) {
+        const Export unsafe_osfile_exports[] = {
+            { "redirect", "redirect" },
+            { "redirectErr", "redirectErr" }
+        };
+
+        for (auto pair : unsafe_osfile_exports) {
+            if (!CreateAlias(cx, pair.dst, osfile, pair.src))
                 return false;
         }
     }
 
     return true;
 }
 
 } // namespace shell
--- a/js/src/shell/jsshell.cpp
+++ b/js/src/shell/jsshell.cpp
@@ -13,62 +13,97 @@
 
 #include "vm/StringBuffer.h"
 
 using namespace JS;
 
 namespace js {
 namespace shell {
 
+// Generate 'usage' and 'help' properties for the given object.
+// JS_DefineFunctionsWithHelp will define individual function objects with both
+// of those properties (eg getpid.usage = "getpid()" and getpid.help = "return
+// the process id"). This function will generate strings for an "interface
+// object", eg os.file, which contains some number of functions.
+//
+// .usage will be set to "<name> - interface object".
+//
+// .help will be set to a newline-separated list of functions that have either
+// 'help' or 'usage' properties. Functions are described with their usage
+// strings, if they have them, else with just their names.
+//
 bool
 GenerateInterfaceHelp(JSContext* cx, HandleObject obj, const char* name)
 {
     AutoIdVector idv(cx);
     if (!GetPropertyKeys(cx, obj, JSITER_OWNONLY | JSITER_HIDDEN, &idv))
         return false;
 
     StringBuffer buf(cx);
-    if (!buf.append(' '))
+    if (!buf.append(name, strlen(name)) || !buf.append(" - interface object", 19))
         return false;
+    RootedString s(cx, buf.finishString());
+    if (!s || !JS_DefineProperty(cx, obj, "usage", s, 0))
+        return false;
+    buf.clear();
 
+    bool first = true;
     for (size_t i = 0; i < idv.length(); i++) {
+        RootedId id(cx, idv[i]);
         RootedValue v(cx);
-        RootedId id(cx, idv[i]);
         if (!JS_GetPropertyById(cx, obj, id, &v))
             return false;
         if (!v.isObject())
             continue;
-        bool hasHelp = false;
         RootedObject prop(cx, &v.toObject());
-        if (!JS_GetProperty(cx, prop, "usage", &v))
+
+        RootedValue usage(cx);
+        RootedValue help(cx);
+        if (!JS_GetProperty(cx, prop, "usage", &usage))
             return false;
-        if (v.isString())
-            hasHelp = true;
-        if (!JS_GetProperty(cx, prop, "help", &v))
+        if (!JS_GetProperty(cx, prop, "help", &help))
             return false;
-        if (v.isString())
-            hasHelp = true;
-        if (hasHelp) {
-            if (!buf.append(' ') ||
-                !buf.append(name, strlen(name)) ||
-                !buf.append('.') ||
-                !buf.append(JSID_TO_FLAT_STRING(id)))
-            {
-                return false;
-            }
-        }
+        if (!usage.isString() && !help.isString())
+            continue;
+
+        if (!first && !buf.append("\n"))
+            return false;
+        first = false;
+
+        if (!buf.append("  ", 2))
+            return false;
+
+        if (!buf.append(usage.isString() ? usage.toString() : JSID_TO_FLAT_STRING(id)))
+            return false;
     }
 
-    RootedString s(cx, buf.finishString());
+    s = buf.finishString();
     if (!s || !JS_DefineProperty(cx, obj, "help", s, 0))
         return false;
 
-    if (!buf.append(name, strlen(name)) || !buf.append(" - interface object", 20))
+    return true;
+}
+
+bool
+CreateAlias(JSContext* cx, const char* dstName, JS::HandleObject namespaceObj, const char* srcName)
+{
+    RootedObject global(cx, JS_GetGlobalForObject(cx, namespaceObj));
+    if (!global)
         return false;
-    s = buf.finishString();
-    if (!s || !JS_DefineProperty(cx, obj, "usage", s, 0))
+
+    RootedValue val(cx);
+    if (!JS_GetProperty(cx, namespaceObj, srcName, &val))
+        return false;
+
+    if (!val.isObject()) {
+        JS_ReportErrorASCII(cx, "attempted to alias nonexistent function");
+        return false;
+    }
+    
+    RootedObject function(cx, &val.toObject());
+    if (!JS_DefineProperty(cx, global, dstName, function, 0))
         return false;
 
     return true;
 }
 
 } // namespace shell
 } // namespace js
--- a/js/src/shell/jsshell.h
+++ b/js/src/shell/jsshell.h
@@ -73,12 +73,24 @@ struct RCFile {
     // Starts out with a ref count of zero.
     static RCFile* create(JSContext* cx, const char* filename, const char* mode);
 
     void close();
     bool isOpen() const { return fp; }
     bool release();
 };
 
+// Alias the global dstName to namespaceObj.srcName. For example, if dstName is
+// "snarf", namespaceObj represents "os.file", and srcName is "readFile", then
+// this is equivalent to the JS code:
+//
+//   snarf = os.file.readFile;
+//
+// This provides a mechanism for namespacing the various JS shell helper
+// functions without breaking backwards compatibility with things that use the
+// global names.
+bool
+CreateAlias(JSContext* cx, const char* dstName, JS::HandleObject namespaceObj, const char* srcName);
+
 } /* namespace shell */
 } /* namespace js */
 
 #endif