Skip to content

Commit 789b0a4

Browse files
erik-krogherik-krogh
authored
Merge pull request #8578 from erik-krogh/labelNaming
JS: update `toString()` on API-graph labels.
2 parents c015ef6 + 8fcbaea commit 789b0a4

File tree

32 files changed

+312
-298
lines changed

32 files changed

+312
-298
lines changed

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

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -383,8 +383,8 @@ module API {
383383
exists(Node pred, Label::ApiLabel lbl, string predpath |
384384
Impl::edge(pred, lbl, this) and
385385
predpath = pred.getAPath(length - 1) and
386-
exists(string space | if length = 1 then space = "" else space = " " |
387-
result = "(" + lbl + space + predpath + ")" and
386+
exists(string dot | if length = 1 then dot = "" else dot = "." |
387+
result = predpath + dot + lbl and
388388
// avoid producing strings longer than 1MB
389389
result.length() < 1000 * 1000
390390
)
@@ -1330,22 +1330,22 @@ module API {
13301330
/** Gets the EntryPoint associated with this label. */
13311331
API::EntryPoint getEntryPoint() { result = e }
13321332

1333-
override string toString() { result = e }
1333+
override string toString() { result = "getASuccessor(Label::entryPoint(\"" + e + "\"))" }
13341334
}
13351335

13361336
/** A label that gets a promised value. */
13371337
class LabelPromised extends ApiLabel, MkLabelPromised {
1338-
override string toString() { result = "promised" }
1338+
override string toString() { result = "getPromised()" }
13391339
}
13401340

13411341
/** A label that gets a rejected promise. */
13421342
class LabelPromisedError extends ApiLabel, MkLabelPromisedError {
1343-
override string toString() { result = "promisedError" }
1343+
override string toString() { result = "getPromisedError()" }
13441344
}
13451345

13461346
/** A label that gets the return value of a function. */
13471347
class LabelReturn extends ApiLabel, MkLabelReturn {
1348-
override string toString() { result = "return" }
1348+
override string toString() { result = "getReturn()" }
13491349
}
13501350

13511351
/** A label for a module. */
@@ -1357,12 +1357,13 @@ module API {
13571357
/** Gets the module associated with this label. */
13581358
string getMod() { result = mod }
13591359

1360-
override string toString() { result = "module " + mod }
1360+
// moduleImport is not neccesarilly the predicate to use, but it's close enough for most cases.
1361+
override string toString() { result = "moduleImport(\"" + mod + "\")" }
13611362
}
13621363

13631364
/** A label that gets an instance from a `new` call. */
13641365
class LabelInstance extends ApiLabel, MkLabelInstance {
1365-
override string toString() { result = "instance" }
1366+
override string toString() { result = "getInstance()" }
13661367
}
13671368

13681369
/** A label for the member named `prop`. */
@@ -1374,14 +1375,14 @@ module API {
13741375
/** Gets the property associated with this label. */
13751376
string getProperty() { result = prop }
13761377

1377-
override string toString() { result = "member " + prop }
1378+
override string toString() { result = "getMember(\"" + prop + "\")" }
13781379
}
13791380

13801381
/** A label for a member with an unknown name. */
13811382
class LabelUnknownMember extends ApiLabel, MkLabelUnknownMember {
13821383
LabelUnknownMember() { this = MkLabelUnknownMember() }
13831384

1384-
override string toString() { result = "member *" }
1385+
override string toString() { result = "getUnknownMember()" }
13851386
}
13861387

13871388
/** A label for parameter `i`. */
@@ -1390,30 +1391,30 @@ module API {
13901391

13911392
LabelParameter() { this = MkLabelParameter(i) }
13921393

1393-
override string toString() { result = "parameter " + i }
1394+
override string toString() { result = "getParameter(" + i + ")" }
13941395

13951396
/** Gets the index of the parameter for this label. */
13961397
int getIndex() { result = i }
13971398
}
13981399

13991400
/** A label for the receiver of call, that is, the value passed as `this`. */
14001401
class LabelReceiver extends ApiLabel, MkLabelReceiver {
1401-
override string toString() { result = "receiver" }
1402+
override string toString() { result = "getReceiver()" }
14021403
}
14031404

14041405
/** A label for a class decorated by the current value. */
14051406
class LabelDecoratedClass extends ApiLabel, MkLabelDecoratedClass {
1406-
override string toString() { result = "decorated-class" }
1407+
override string toString() { result = "getADecoratedClass()" }
14071408
}
14081409

14091410
/** A label for a method, field, or accessor decorated by the current value. */
14101411
class LabelDecoratedMethod extends ApiLabel, MkLabelDecoratedMember {
1411-
override string toString() { result = "decorated-member" }
1412+
override string toString() { result = "decoratedMember()" }
14121413
}
14131414

14141415
/** A label for a parameter decorated by the current value. */
14151416
class LabelDecoratedParameter extends ApiLabel, MkLabelDecoratedParameter {
1416-
override string toString() { result = "decorated-parameter" }
1417+
override string toString() { result = "decoratedParameter()" }
14171418
}
14181419
}
14191420
}

‎javascript/ql/test/ApiGraphs/VerifyAssertions.qll

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
/**
22
* A test query that verifies assertions about the API graph embedded in source-code comments.
33
*
4-
* An assertion is a comment of the form `def <path>` or `use <path>`, and asserts that
5-
* there is a def/use feature reachable from the root along the given path (described using
6-
* s-expression syntax), and its associated data-flow node must start on the same line as the
7-
* comment.
4+
* An assertion is a comment of the form `def=<path>` or `use=<path>`, and asserts that
5+
* there is a def/use feature reachable from the root along the given path, and its
6+
* associated data-flow node must start on the same line as the comment.
87
*
9-
* We also support negative assertions of the form `!def <path>` or `!use <path>`, which assert
8+
* We also support negative assertions of the form `MISSING: def <path>` or `MISSING: use <path>`, which assert
109
* that there _isn't_ a node with the given path on the same line.
1110
*
1211
* 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) {
3938
* An assertion matching a data-flow node against an API-graph feature.
4039
*/
4140
class Assertion extends Comment {
42-
string polarity;
4341
string expectedKind;
4442
string expectedLoc;
43+
string path;
44+
string polarity;
4545

4646
Assertion() {
4747
exists(string txt, string rex |
4848
txt = this.getText().trim() and
49-
rex = "(!?)(def|use) .*"
49+
rex = ".*?((?:MISSING: )?)(def|use)=([\\w\\(\\)\"\\.\\-\\/\\@\\:]*).*"
5050
|
5151
polarity = txt.regexpCapture(rex, 1) and
5252
expectedKind = txt.regexpCapture(rex, 2) and
53+
path = txt.regexpCapture(rex, 3) and
5354
expectedLoc = this.getFile().getAbsolutePath() + ":" + this.getLocation().getStartLine()
5455
)
5556
}
5657

57-
string getEdgeLabel(int i) { result = this.getText().regexpFind("(?<=\\()[^()]+", i, _).trim() }
58+
string getEdgeLabel(int i) {
59+
// matches a single edge. E.g. `getParameter(1)` or `getMember("foo")`.
60+
// The lookbehind/lookahead ensure that the boundary is correct, that is
61+
// either the edge is next to a ".", or it's the end of the string.
62+
result = path.regexpFind("(?<=\\.|^)([\\w\\(\\)\"\\-\\/\\@\\:]+)(?=\\.|$)", i, _).trim()
63+
}
5864

5965
int getPathLength() { result = max(int i | exists(this.getEdgeLabel(i))) + 1 }
6066

67+
predicate isNegative() { polarity = "MISSING: " }
68+
6169
API::Node lookup(int i) {
62-
i = this.getPathLength() and
70+
i = 0 and
6371
result = API::root()
6472
or
6573
result =
66-
this.lookup(i + 1)
67-
.getASuccessor(any(API::Label::ApiLabel label | label.toString() = this.getEdgeLabel(i)))
74+
this.lookup(i - 1)
75+
.getASuccessor(any(API::Label::ApiLabel label |
76+
label.toString() = this.getEdgeLabel(i - 1)
77+
))
6878
}
6979

70-
predicate isNegative() { polarity = "!" }
80+
API::Node lookup() { result = this.lookup(this.getPathLength()) }
7181

72-
predicate holds() { getLoc(getNode(this.lookup(0), expectedKind)) = expectedLoc }
82+
predicate holds() { getLoc(getNode(this.lookup(), expectedKind)) = expectedLoc }
7383

7484
string tryExplainFailure() {
7585
exists(int i, API::Node nd, string prefix, string suffix |
7686
nd = this.lookup(i) and
77-
i > 0 and
78-
not exists(this.lookup([0 .. i - 1])) and
79-
prefix = nd + " has no outgoing edge labelled " + this.getEdgeLabel(i - 1) + ";" and
87+
i < getPathLength() and
88+
not exists(this.lookup([i + 1 .. getPathLength()])) and
89+
prefix = nd + " has no outgoing edge labelled " + this.getEdgeLabel(i) + ";" and
8090
if exists(nd.getASuccessor())
8191
then
8292
suffix =
@@ -91,13 +101,13 @@ class Assertion extends Comment {
91101
result = prefix + " " + suffix
92102
)
93103
or
94-
exists(API::Node nd, string kind | nd = this.lookup(0) |
104+
exists(API::Node nd, string kind | nd = this.lookup() |
95105
exists(getNode(nd, kind)) and
96106
not exists(getNode(nd, expectedKind)) and
97107
result = "Expected " + expectedKind + " node, but found " + kind + " node."
98108
)
99109
or
100-
exists(DataFlow::Node nd | nd = getNode(this.lookup(0), expectedKind) |
110+
exists(DataFlow::Node nd | nd = getNode(this.lookup(), expectedKind) |
101111
not getLoc(nd) = expectedLoc and
102112
result = "Node not found on this line (but there is one on line " + min(getLoc(nd)) + ")."
103113
)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
const assert = require("assert");
22

33
let o = {
4-
foo: 23 /* def (member foo (parameter 0 (member equal (member exports (module assert))))) */
4+
foo: 23 // def=moduleImport("assert").getMember("exports").getMember("equal").getParameter(0).getMember("foo")
55
};
66
assert.equal(o, o);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
const fs = require('fs-extra');
22

33
module.exports.foo = async function foo() {
4-
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))))) */
4+
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() */
55
};

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@ async function readFileUtf8(path: string): Promise<string> {
55
}
66

77
async function test(path: string) {
8-
await readFileUtf8(path); /* use (promised (return (member readFile (member exports (module fs/promises))))) */
8+
await readFileUtf8(path); /* use=moduleImport("fs/promises").getMember("exports").getMember("readFile").getReturn() */
99
}

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,24 @@
11
import bar from 'foo';
22

33
let boundbar = bar.bind(
4-
"receiver", // def (receiver (member default (member exports (module foo))))
5-
"firstarg" // def (parameter 0 (member default (member exports (module foo))))
4+
"receiver", // def=moduleImport("foo").getMember("exports").getMember("default").getReceiver()
5+
"firstarg" // def=moduleImport("foo").getMember("exports").getMember("default").getParameter(0)
66
);
77
boundbar(
8-
"secondarg" // def (parameter 1 (member default (member exports (module foo))))
8+
"secondarg" // def=moduleImport("foo").getMember("exports").getMember("default").getParameter(1)
99
)
1010

1111
let boundbar2 = boundbar.bind(
12-
"ignored", // !def (receiver (member default (member exports (module foo))))
13-
"othersecondarg" // def (parameter 1 (member default (member exports (module foo))))
12+
"ignored", // MISSING: def=moduleImport("foo").getMember("exports)".getMember("default").getReceiver()
13+
"othersecondarg" // def=moduleImport("foo").getMember("exports").getMember("default").getParameter(1)
1414
)
1515
boundbar2(
16-
"thirdarg" // def (parameter 2 (member default (member exports (module foo))))
16+
"thirdarg" // def=moduleImport("foo").getMember("exports").getMember("default").getParameter(2)
1717
)
1818

1919
let bar2 = bar;
2020
for (var i = 0; i < 2; ++i)
2121
bar2 = bar2.bind(
2222
null,
23-
i /* def (parameter 1 (member default (member exports (module foo)))) */ /* def (parameter 9 (member default (member exports (module foo)))) */
23+
i /* def=moduleImport("foo").getMember("exports").getMember("default").getParameter(1) */ /* def=moduleImport("foo").getMember("exports").getMember("default").getParameter(9) */
2424
);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,5 @@ const fs = require('fs');
33
exports.foo = function (cb) {
44
if (!cb)
55
cb = function () { };
6-
cb(fs.readFileSync("/etc/passwd")); /* def (parameter 0 (parameter 0 (member foo (member exports (module branching-flow))))) */
6+
cb(fs.readFileSync("/etc/passwd")); /* def=moduleImport("branching-flow").getMember("exports").getMember("foo").getParameter(0).getParameter(0) */
77
};

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,19 @@ util.inherits(MyStream, EventEmitter);
99

1010
MyStream.prototype.write = (data) => this.emit('data', data);
1111

12-
function MyOtherStream() { /* use (instance (member MyOtherStream (member exports (module classes)))) */
12+
function MyOtherStream() { /* use=moduleImport("classes").getMember("exports").getMember("MyOtherStream").getInstance() */
1313
EventEmitter.call(this);
1414
}
1515

1616
util.inherits(MyOtherStream, EventEmitter);
1717

18-
MyOtherStream.prototype.write = function (data) { /* use (instance (member MyOtherStream (member exports (module classes)))) */
18+
MyOtherStream.prototype.write = function (data) { /* use=moduleImport("classes").getMember("exports").getMember("MyOtherStream").getInstance() */
1919
this.emit('data', data);
2020
return this;
2121
};
2222

23-
MyOtherStream.prototype.instanceProp = 1; /* def (member instanceProp (instance (member MyOtherStream (member exports (module classes))))) */
23+
MyOtherStream.prototype.instanceProp = 1; /* def=moduleImport("classes").getMember("exports").getMember("MyOtherStream").getInstance().getMember("instanceProp") */
2424

25-
MyOtherStream.classProp = 1; /* def (member classProp (member MyOtherStream (member exports (module classes)))) */
25+
MyOtherStream.classProp = 1; /* def=moduleImport("classes").getMember("exports").getMember("MyOtherStream").getMember("classProp") */
2626

2727
module.exports.MyOtherStream = MyOtherStream;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
export class A {
2-
constructor(x) { /* use (parameter 0 (member A (member exports (module ctor-arg)))) */
2+
constructor(x) { /* use=moduleImport("ctor-arg").getMember("exports").getMember("A").getParameter(0) */
33
console.log(x);
44
}
55
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
module.exports = CustomEntryPoint.foo; /* use (member foo (CustomEntryPoint)) */
1+
module.exports = CustomEntryPoint.foo; /* use=getASuccessor(Label::entryPoint("CustomEntryPoint")) */

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
const foo = require("foo");
22

33
while(foo)
4-
foo = foo.foo; /* use (member foo (member exports (module foo))) */ /* use (member foo (member foo (member exports (module foo)))) */
4+
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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@ const MyStream = require('classes').MyStream;
22

33
var s = new MyStream();
44
for (let m of ["write"])
5-
s[m]("Hello, world!"); /* use (member * (instance (member MyStream (member exports (module classes))))) */
5+
s[m]("Hello, world!"); /* use=moduleImport("classes").getMember("exports").getMember("MyStream").getInstance().getUnknownMember() */

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
anotherUnknownFunction().foo = 42; /* !def (member foo (member exports (module imprecise-export))) */
1+
anotherUnknownFunction().foo = 42; /* MISSING: def=moduleExport("imprecise-export").getMember("exports").getMember("foo") */
22

33
module.exports = unknownFunction();

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
const http = require('http');
22
let req = http.get(url, cb);
33
req.on('connect', (
4-
req, /* use (parameter 0 (parameter 1 (member on (return (member get (member exports (module http))))))) */
4+
req, /* use=moduleImport("http").getMember("exports").getMember("get").getReturn().getMember("on").getParameter(1).getParameter(0) */
55
clientSocket, head) => { /* ... */ });
66
req.on('information', (
7-
info /* use (parameter 0 (parameter 1 (member on (return (member get (member exports (module http))))))) */
7+
info /* use=moduleImport("http").getMember("exports").getMember("get").getReturn().getMember("on").getParameter(1).getParameter(0) */
88
) => { /* ... */ });
99

10-
req.on('connect', () => { }) /* def (parameter 0 (member on (return (member get (member exports (module http)))))) */
11-
.on('information', () => { }) /* def (parameter 0 (member on (return (member on (return (member get (member exports (module http)))))))) */;
10+
req.on('connect', () => { }) /* def=moduleImport("http").getMember("exports").getMember("get").getReturn().getMember("on").getParameter(0) */
11+
.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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
import foo from "@myorg/myotherpkg";
2-
foo(); /* use (member default (member exports (module @myorg/myotherpkg))) */
2+
foo(); /* use=moduleImport("@myorg/myotherpkg").getMember("exports").getMember("default") */

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
module.exports.foo = function (x) { /* use (parameter 0 (member foo (member exports (module nested-property-export)))) */
1+
module.exports.foo = function (x) { /* use=moduleImport("nested-property-export").getMember("exports").getMember("foo").getParameter(0) */
22
return x;
33
};
44

5-
module.exports.foo.bar = function (y) { /* use (parameter 0 (member bar (member foo (member exports (module nested-property-export))))) */
5+
module.exports.foo.bar = function (y) { /* use=moduleImport("nested-property-export").getMember("exports").getMember("foo").getMember("bar").getParameter(0) */
66
return y;
77
};

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ const express = require('express');
22

33
var app1 = new express();
44
app1.get('/',
5-
(req, res) => res.send('Hello World!') /* def (parameter 1 (member get (instance (member exports (module express))))) */
5+
(req, res) => res.send('Hello World!') /* def=moduleImport("express").getMember("exports").getInstance().getMember("get").getParameter(1) */
66
);
77

88
function makeApp() {
@@ -11,5 +11,5 @@ function makeApp() {
1111

1212
var app2 = makeApp();
1313
app2.get('/',
14-
(req, res) => res.send('Hello World!') /* def (parameter 1 (member get (instance (member exports (module express))))) */
14+
(req, res) => res.send('Hello World!') /* def=moduleImport("express").getMember("exports").getInstance().getMember("get").getParameter(1) */
1515
);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ const cp = require('child_process');
22

33
module.exports = function () {
44
return cp.spawn.bind(
5-
cp, // def (receiver (member spawn (member exports (module child_process))))
6-
"cat" // def (parameter 0 (member spawn (member exports (module child_process))))
5+
cp, // def=moduleImport("child_process").getMember("exports").getMember("spawn").getReceiver()
6+
"cat" // def=moduleImport("child_process").getMember("exports").getMember("spawn").getParameter(0)
77
);
88
};

0 commit comments

Comments
 (0)