JS: update toString() on API-graph labels.

javascript/ql/lib/semmle/javascript/ApiGraphs.qll

@@ -383,8 +383,8 @@ module API {
       exists(Node pred, Label::ApiLabel lbl, string predpath |
         Impl::edge(pred, lbl, this) and
         predpath = pred.getAPath(length - 1) and
-        exists(string space | if length = 1 then space = "" else space = " " |
-          result = "(" + lbl + space + predpath + ")" and
+        exists(string dot | if length = 1 then dot = "" else dot = "." |
+          result = predpath + dot + lbl and
           // avoid producing strings longer than 1MB
           result.length() < 1000 * 1000
         )
@@ -1330,22 +1330,22 @@ module API {
         /** Gets the EntryPoint associated with this label. */
         API::EntryPoint getEntryPoint() { result = e }
 
-        override string toString() { result = e }
+        override string toString() { result = "getASuccessor(Label::entryPoint(\"" + e + "\"))" }
       }
 
       /** A label that gets a promised value. */
       class LabelPromised extends ApiLabel, MkLabelPromised {
-        override string toString() { result = "promised" }
+        override string toString() { result = "getPromised()" }
       }
 
       /** A label that gets a rejected promise. */
       class LabelPromisedError extends ApiLabel, MkLabelPromisedError {
-        override string toString() { result = "promisedError" }
+        override string toString() { result = "getPromisedError()" }
       }
 
       /** A label that gets the return value of a function. */
       class LabelReturn extends ApiLabel, MkLabelReturn {
-        override string toString() { result = "return" }
+        override string toString() { result = "getReturn()" }
       }
 
       /** A label for a module. */
@@ -1357,12 +1357,13 @@ module API {
         /** Gets the module associated with this label. */
         string getMod() { result = mod }
 
-        override string toString() { result = "module " + mod }
+        // moduleImport is not neccesarilly the predicate to use, but it's close enough for most cases.
+        override string toString() { result = "moduleImport(\"" + mod + "\")" }
       }
 
       /** A label that gets an instance from a `new` call. */
       class LabelInstance extends ApiLabel, MkLabelInstance {
-        override string toString() { result = "instance" }
+        override string toString() { result = "getInstance()" }
       }
 
       /** A label for the member named `prop`. */
@@ -1374,14 +1375,14 @@ module API {
         /** Gets the property associated with this label. */
         string getProperty() { result = prop }
 
-        override string toString() { result = "member " + prop }
+        override string toString() { result = "getMember(\"" + prop + "\")" }
       }
 
       /** A label for a member with an unknown name. */
       class LabelUnknownMember extends ApiLabel, MkLabelUnknownMember {
         LabelUnknownMember() { this = MkLabelUnknownMember() }
 
-        override string toString() { result = "member *" }
+        override string toString() { result = "getUnknownMember()" }
       }
 
       /** A label for parameter `i`. */
@@ -1390,30 +1391,30 @@ module API {
 
         LabelParameter() { this = MkLabelParameter(i) }
 
-        override string toString() { result = "parameter " + i }
+        override string toString() { result = "getParameter(" + i + ")" }
 
         /** Gets the index of the parameter for this label. */
         int getIndex() { result = i }
       }
 
       /** A label for the receiver of call, that is, the value passed as `this`. */
       class LabelReceiver extends ApiLabel, MkLabelReceiver {
-        override string toString() { result = "receiver" }
+        override string toString() { result = "getReceiver()" }
       }
 
       /** A label for a class decorated by the current value. */
       class LabelDecoratedClass extends ApiLabel, MkLabelDecoratedClass {
-        override string toString() { result = "decorated-class" }
+        override string toString() { result = "getADecoratedClass()" }
       }
 
       /** A label for a method, field, or accessor decorated by the current value. */
       class LabelDecoratedMethod extends ApiLabel, MkLabelDecoratedMember {
-        override string toString() { result = "decorated-member" }
+        override string toString() { result = "decoratedMember()" }
       }
 
       /** A label for a parameter decorated by the current value. */
       class LabelDecoratedParameter extends ApiLabel, MkLabelDecoratedParameter {
-        override string toString() { result = "decorated-parameter" }
+        override string toString() { result = "decoratedParameter()" }
       }
     }
   }

javascript/ql/test/ApiGraphs/VerifyAssertions.qll

@@ -1,12 +1,11 @@
 /**
  * A test query that verifies assertions about the API graph embedded in source-code comments.
  *
- * An assertion is a comment of the form `def <path>` or `use <path>`, and asserts that
- * there is a def/use feature reachable from the root along the given path (described using
- * s-expression syntax), and its associated data-flow node must start on the same line as the
- * comment.
+ * An assertion is a comment of the form `def=<path>` or `use=<path>`, and asserts that
+ * there is a def/use feature reachable from the root along the given path, and its
+ * associated data-flow node must start on the same line as the comment.
  *
- * We also support negative assertions of the form `!def <path>` or `!use <path>`, which assert
+ * We also support negative assertions of the form `MISSING: def <path>` or `MISSING: use <path>`, which assert
  * that there _isn't_ a node with the given path on the same line.
  *
  * The query only produces output for failed assertions, meaning that it should have no output
@@ -39,44 +38,55 @@ private string getLoc(DataFlow::Node nd) {
  * An assertion matching a data-flow node against an API-graph feature.
  */
 class Assertion extends Comment {
-  string polarity;
   string expectedKind;
   string expectedLoc;
+  string path;
+  string polarity;
 
   Assertion() {
     exists(string txt, string rex |
       txt = this.getText().trim() and
-      rex = "(!?)(def|use) .*"
+      rex = ".*?((?:MISSING: )?)(def|use)=([\\w\\(\\)\"\\.\\-\\/\\@\\:]*).*"
     |
       polarity = txt.regexpCapture(rex, 1) and
       expectedKind = txt.regexpCapture(rex, 2) and
+      path = txt.regexpCapture(rex, 3) and
       expectedLoc = this.getFile().getAbsolutePath() + ":" + this.getLocation().getStartLine()
     )
   }
 
-  string getEdgeLabel(int i) { result = this.getText().regexpFind("(?<=\\()[^()]+", i, _).trim() }
+  string getEdgeLabel(int i) {
+    // matches a single edge. E.g. `getParameter(1)` or `getMember("foo")`.
+    // The lookbehind/lookahead ensure that the boundary is correct, that is
+    // either the edge is next to a ".", or it's the end of the string.
+    result = path.regexpFind("(?<=\\.|^)([\\w\\(\\)\"\\-\\/\\@\\:]+)(?=\\.|$)", i, _).trim()
+  }
 
   int getPathLength() { result = max(int i | exists(this.getEdgeLabel(i))) + 1 }
 
+  predicate isNegative() { polarity = "MISSING: " }
+
   API::Node lookup(int i) {
-    i = this.getPathLength() and
+    i = 0 and
     result = API::root()
     or
     result =
-      this.lookup(i + 1)
-          .getASuccessor(any(API::Label::ApiLabel label | label.toString() = this.getEdgeLabel(i)))
+      this.lookup(i - 1)
+          .getASuccessor(any(API::Label::ApiLabel label |
+              label.toString() = this.getEdgeLabel(i - 1)
+            ))
   }
 
-  predicate isNegative() { polarity = "!" }
+  API::Node lookup() { result = this.lookup(this.getPathLength()) }
 
-  predicate holds() { getLoc(getNode(this.lookup(0), expectedKind)) = expectedLoc }
+  predicate holds() { getLoc(getNode(this.lookup(), expectedKind)) = expectedLoc }
 
   string tryExplainFailure() {
     exists(int i, API::Node nd, string prefix, string suffix |
       nd = this.lookup(i) and
-      i > 0 and
-      not exists(this.lookup([0 .. i - 1])) and
-      prefix = nd + " has no outgoing edge labelled " + this.getEdgeLabel(i - 1) + ";" and
+      i < getPathLength() and
+      not exists(this.lookup([i + 1 .. getPathLength()])) and
+      prefix = nd + " has no outgoing edge labelled " + this.getEdgeLabel(i) + ";" and
       if exists(nd.getASuccessor())
       then
         suffix =
@@ -91,13 +101,13 @@ class Assertion extends Comment {
       result = prefix + " " + suffix
     )
     or
-    exists(API::Node nd, string kind | nd = this.lookup(0) |
+    exists(API::Node nd, string kind | nd = this.lookup() |
       exists(getNode(nd, kind)) and
       not exists(getNode(nd, expectedKind)) and
       result = "Expected " + expectedKind + " node, but found " + kind + " node."
     )
     or
-    exists(DataFlow::Node nd | nd = getNode(this.lookup(0), expectedKind) |
+    exists(DataFlow::Node nd | nd = getNode(this.lookup(), expectedKind) |
       not getLoc(nd) = expectedLoc and
       result = "Node not found on this line (but there is one on line " + min(getLoc(nd)) + ")."
     )

javascript/ql/test/ApiGraphs/argprops/index.js

@@ -1,6 +1,6 @@
 const assert = require("assert");
 
 let o = {
-    foo: 23 /* def (member foo (parameter 0 (member equal (member exports (module assert))))) */
+    foo: 23 // def=moduleImport("assert").getMember("exports").getMember("equal").getParameter(0).getMember("foo")
 };
 assert.equal(o, o);

javascript/ql/test/ApiGraphs/async-await/index.js

@@ -1,5 +1,5 @@
 const fs = require('fs-extra');
 
 module.exports.foo = async function foo() {
-    return await fs.copy('/tmp/myfile', '/tmp/mynewfile'); /* use (promised (return (member copy (member exports (module fs-extra))))) */ /* def (promised (return (member foo (member exports (module async-await))))) */
+    return await fs.copy('/tmp/myfile', '/tmp/mynewfile'); /* use=moduleImport("fs-extra").getMember("exports").getMember("copy").getReturn().getPromised()*/ /* def=moduleImport("async-await").getMember("exports").getMember("foo").getReturn().getPromised() */
 };

javascript/ql/test/ApiGraphs/async-await/tst.ts

@@ -5,5 +5,5 @@ async function readFileUtf8(path: string): Promise<string> {
 }
 
 async function test(path: string) {
-    await readFileUtf8(path); /* use (promised (return (member readFile (member exports (module fs/promises))))) */
+    await readFileUtf8(path); /* use=moduleImport("fs/promises").getMember("exports").getMember("readFile").getReturn() */
 }

javascript/ql/test/ApiGraphs/bound-args/index.js

@@ -1,24 +1,24 @@
 import bar from 'foo';
 
 let boundbar = bar.bind(
-    "receiver", // def (receiver (member default (member exports (module foo))))
-    "firstarg"  // def (parameter 0 (member default (member exports (module foo))))
+    "receiver", // def=moduleImport("foo").getMember("exports").getMember("default").getReceiver()
+    "firstarg"  // def=moduleImport("foo").getMember("exports").getMember("default").getParameter(0)
 );
 boundbar(
-    "secondarg" // def (parameter 1 (member default (member exports (module foo))))
+    "secondarg" // def=moduleImport("foo").getMember("exports").getMember("default").getParameter(1)
 )
 
 let boundbar2 = boundbar.bind(
-    "ignored", // !def (receiver (member default (member exports (module foo))))
-    "othersecondarg" // def (parameter 1 (member default (member exports (module foo))))
+    "ignored", // MISSING: def=moduleImport("foo").getMember("exports)".getMember("default").getReceiver()
+    "othersecondarg" // def=moduleImport("foo").getMember("exports").getMember("default").getParameter(1)
 )
 boundbar2(
-    "thirdarg" // def (parameter 2 (member default (member exports (module foo))))
+    "thirdarg" // def=moduleImport("foo").getMember("exports").getMember("default").getParameter(2)
 )
 
 let bar2 = bar;
 for (var i = 0; i < 2; ++i)
     bar2 = bar2.bind(
         null,
-        i /* def (parameter 1 (member default (member exports (module foo)))) */ /* def (parameter 9 (member default (member exports (module foo)))) */
+        i /* def=moduleImport("foo").getMember("exports").getMember("default").getParameter(1) */ /* def=moduleImport("foo").getMember("exports").getMember("default").getParameter(9) */
     );

javascript/ql/test/ApiGraphs/branching-flow/index.js

@@ -3,5 +3,5 @@ const fs = require('fs');
 exports.foo = function (cb) {
     if (!cb)
         cb = function () { };
-    cb(fs.readFileSync("/etc/passwd")); /* def (parameter 0 (parameter 0 (member foo (member exports (module branching-flow))))) */
+    cb(fs.readFileSync("/etc/passwd")); /* def=moduleImport("branching-flow").getMember("exports").getMember("foo").getParameter(0).getParameter(0) */
 };

javascript/ql/test/ApiGraphs/classes/classes.js

@@ -9,19 +9,19 @@ util.inherits(MyStream, EventEmitter);
 
 MyStream.prototype.write = (data) => this.emit('data', data);
 
-function MyOtherStream() { /* use (instance (member MyOtherStream (member exports (module classes)))) */
+function MyOtherStream() { /* use=moduleImport("classes").getMember("exports").getMember("MyOtherStream").getInstance() */
     EventEmitter.call(this);
 }
 
 util.inherits(MyOtherStream, EventEmitter);
 
-MyOtherStream.prototype.write = function (data) { /* use (instance (member MyOtherStream (member exports (module classes)))) */
+MyOtherStream.prototype.write = function (data) { /* use=moduleImport("classes").getMember("exports").getMember("MyOtherStream").getInstance() */
     this.emit('data', data);
     return this;
 };
 
-MyOtherStream.prototype.instanceProp = 1; /* def (member instanceProp (instance (member MyOtherStream (member exports (module classes))))) */
+MyOtherStream.prototype.instanceProp = 1; /* def=moduleImport("classes").getMember("exports").getMember("MyOtherStream").getInstance().getMember("instanceProp") */
 
-MyOtherStream.classProp = 1; /* def (member classProp (member MyOtherStream (member exports (module classes)))) */
+MyOtherStream.classProp = 1; /* def=moduleImport("classes").getMember("exports").getMember("MyOtherStream").getMember("classProp") */
 
 module.exports.MyOtherStream = MyOtherStream;

javascript/ql/test/ApiGraphs/ctor-arg/index.js

@@ -1,5 +1,5 @@
 export class A {
-    constructor(x) { /* use (parameter 0 (member A (member exports (module ctor-arg)))) */
+    constructor(x) { /* use=moduleImport("ctor-arg").getMember("exports").getMember("A").getParameter(0) */
         console.log(x);
     }
 }

javascript/ql/test/ApiGraphs/custom-entry-point/index.js

@@ -1 +1 @@
-module.exports = CustomEntryPoint.foo; /* use (member foo (CustomEntryPoint)) */
+module.exports = CustomEntryPoint.foo; /* use=getASuccessor(Label::entryPoint("CustomEntryPoint")) */

javascript/ql/test/ApiGraphs/cyclic/index.js

@@ -1,4 +1,4 @@
 const foo = require("foo");
 
 while(foo)
-  foo = foo.foo; /* use (member foo (member exports (module foo))) */ /* use (member foo (member foo (member exports (module foo)))) */
+  foo = foo.foo; /* use=moduleImport("foo").getMember("exports").getMember("foo") */ /* use=moduleImport("foo").getMember("exports").getMember("foo").getMember("foo") */

javascript/ql/test/ApiGraphs/dynamic-prop-read/index.js

@@ -2,4 +2,4 @@ const MyStream = require('classes').MyStream;
 
 var s = new MyStream();
 for (let m of ["write"])
-    s[m]("Hello, world!"); /* use (member * (instance (member MyStream (member exports (module classes))))) */
+    s[m]("Hello, world!"); /* use=moduleImport("classes").getMember("exports").getMember("MyStream").getInstance().getUnknownMember() */

javascript/ql/test/ApiGraphs/imprecise-export/index.js

@@ -1,3 +1,3 @@
-anotherUnknownFunction().foo = 42; /* !def (member foo (member exports (module imprecise-export))) */
+anotherUnknownFunction().foo = 42; /* MISSING: def=moduleExport("imprecise-export").getMember("exports").getMember("foo") */
 
 module.exports = unknownFunction();

javascript/ql/test/ApiGraphs/imprecision/index.js

@@ -1,11 +1,11 @@
 const http = require('http');
 let req = http.get(url, cb);
 req.on('connect', (
-    req, /* use (parameter 0 (parameter 1 (member on (return (member get (member exports (module http))))))) */
+    req, /* use=moduleImport("http").getMember("exports").getMember("get").getReturn().getMember("on").getParameter(1).getParameter(0) */
     clientSocket, head) => { /* ...  */ });
 req.on('information', (
-    info /* use (parameter 0 (parameter 1 (member on (return (member get (member exports (module http))))))) */
+    info /* use=moduleImport("http").getMember("exports").getMember("get").getReturn().getMember("on").getParameter(1).getParameter(0) */
     ) => { /* ... */ });
 
-req.on('connect', () => { }) /* def (parameter 0 (member on (return (member get (member exports (module http)))))) */
-   .on('information', () => { }) /* def (parameter 0 (member on (return (member on (return (member get (member exports (module http)))))))) */;
+req.on('connect', () => { }) /* def=moduleImport("http").getMember("exports").getMember("get").getReturn().getMember("on").getParameter(0) */
+   .on('information', () => { }) /* def=moduleImport("http").getMember("exports").getMember("get").getReturn().getMember("on").getReturn().getMember("on").getParameter(0) */;

javascript/ql/test/ApiGraphs/namespaced-package/index.js

@@ -1,2 +1,2 @@
 import foo from "@myorg/myotherpkg";
-foo(); /* use (member default (member exports (module @myorg/myotherpkg))) */
+foo(); /* use=moduleImport("@myorg/myotherpkg").getMember("exports").getMember("default") */

javascript/ql/test/ApiGraphs/nested-property-export/index.js

@@ -1,7 +1,7 @@
-module.exports.foo = function (x) { /* use (parameter 0 (member foo (member exports (module nested-property-export)))) */
+module.exports.foo = function (x) { /* use=moduleImport("nested-property-export").getMember("exports").getMember("foo").getParameter(0) */
     return x;
 };
 
-module.exports.foo.bar = function (y) { /* use (parameter 0 (member bar (member foo (member exports (module nested-property-export))))) */
+module.exports.foo.bar = function (y) { /* use=moduleImport("nested-property-export").getMember("exports").getMember("foo").getMember("bar").getParameter(0) */
     return y;
 };

javascript/ql/test/ApiGraphs/nonlocal/index.js

@@ -2,7 +2,7 @@ const express = require('express');
 
 var app1 = new express();
 app1.get('/',
-    (req, res) => res.send('Hello World!') /* def (parameter 1 (member get (instance (member exports (module express))))) */
+    (req, res) => res.send('Hello World!') /* def=moduleImport("express").getMember("exports").getInstance().getMember("get").getParameter(1) */
 );
 
 function makeApp() {
@@ -11,5 +11,5 @@ function makeApp() {
 
 var app2 = makeApp();
 app2.get('/',
-    (req, res) => res.send('Hello World!') /* def (parameter 1 (member get (instance (member exports (module express))))) */
+    (req, res) => res.send('Hello World!') /* def=moduleImport("express").getMember("exports").getInstance().getMember("get").getParameter(1) */
 );

javascript/ql/test/ApiGraphs/partial-invoke/index.js

@@ -2,7 +2,7 @@ const cp = require('child_process');
 
 module.exports = function () {
     return cp.spawn.bind(
-        cp,   // def (receiver (member spawn (member exports (module child_process))))
-        "cat" // def (parameter 0 (member spawn (member exports (module child_process))))
+        cp,   // def=moduleImport("child_process").getMember("exports").getMember("spawn").getReceiver()
+        "cat" // def=moduleImport("child_process").getMember("exports").getMember("spawn").getParameter(0)
     );
 };