Skip to content

Commit 247848c

Browse files
authored
Merge pull request #1577 from asger-semmle/infername
Approved by xiemaisi
2 parents 12c906c + cee7421 commit 247848c

File tree

11 files changed

+121
-37
lines changed

11 files changed

+121
-37
lines changed

change-notes/1.22/analysis-javascript.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,5 @@
2727

2828
## Changes to QL libraries
2929

30+
- The `getName()` predicate on functions and classes now gets a name
31+
inferred from the context if the function or class was not declared with a name.

javascript/ql/src/semmle/javascript/Classes.qll

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,15 @@ class ClassOrInterface extends @classorinterface, TypeParameterized {
3232
/** Gets the identifier naming the declared type, if any. */
3333
Identifier getIdentifier() { none() } // Overridden in subtypes.
3434

35-
/** Gets the name of the defined class or interface, if any. */
36-
string getName() { result = getIdentifier().getName() }
35+
/**
36+
* Gets the name of the defined class or interface, possibly inferred
37+
* from the context if this is an anonymous class expression.
38+
*
39+
* Has no result if no name could be determined.
40+
*/
41+
string getName() {
42+
result = getIdentifier().getName() // Overridden in ClassExpr
43+
}
3744

3845
/** Gets the nearest enclosing function or toplevel in which this class or interface occurs. */
3946
StmtContainer getContainer() { result = this.(ExprOrStmt).getContainer() }
@@ -203,8 +210,8 @@ class ClassDefinition extends @classdefinition, ClassOrInterface, AST::ValueNode
203210
*/
204211
private string inferNameFromVarDef() {
205212
// in ambiguous cases like `let C = class D {}`, prefer `D` to `C`
206-
if exists(getName())
207-
then result = "class " + getName()
213+
if exists(getIdentifier())
214+
then result = "class " + getIdentifier().getName()
208215
else
209216
exists(VarDef vd | this = vd.getSource() |
210217
result = "class " + vd.getTarget().(VarRef).getName()
@@ -272,6 +279,26 @@ class ClassDeclStmt extends @classdeclstmt, ClassDefinition, Stmt {
272279
* ```
273280
*/
274281
class ClassExpr extends @classexpr, ClassDefinition, Expr {
282+
override string getName() {
283+
result = ClassDefinition.super.getName()
284+
or
285+
not exists(getIdentifier()) and
286+
(
287+
exists(VarDef vd | this = vd.getSource() | result = vd.getTarget().(VarRef).getName())
288+
or
289+
exists(ValueProperty p |
290+
this = p.getInit() and
291+
result = p.getName()
292+
)
293+
or
294+
exists(AssignExpr assign, PropAccess prop |
295+
this = assign.getRhs().getUnderlyingValue() and
296+
prop = assign.getLhs() and
297+
result = prop.getPropertyName()
298+
)
299+
)
300+
}
301+
275302
override predicate isImpure() { none() }
276303

277304
/** Gets the nearest enclosing function or toplevel in which this class expression occurs. */

javascript/ql/src/semmle/javascript/Expr.qll

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -751,9 +751,6 @@ class SpreadProperty extends Property {
751751
* ```
752752
*/
753753
class FunctionExpr extends @functionexpr, Expr, Function {
754-
/** Gets the name of this function expression, if any. */
755-
override string getName() { result = getId().getName() }
756-
757754
/** Holds if this function expression is a property setter. */
758755
predicate isSetter() { exists(PropertySetter s | s.getInit() = this) }
759756

javascript/ql/src/semmle/javascript/Functions.qll

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,35 @@ class Function extends @function, Parameterized, TypeParameterized, StmtContaine
8080
/** Gets the identifier specifying the name of this function, if any. */
8181
VarDecl getId() { result = getChildExpr(-1) }
8282

83-
/** Gets the name of this function, if any. */
84-
string getName() { result = getId().getName() }
83+
/**
84+
* Gets the name of this function if it has one, or a name inferred from its context.
85+
*
86+
* For named functions such as `function f() { ... }`, this is just the declared
87+
* name. For functions assigned to variables or properties (including class
88+
* members), this is the name of the variable or property. If no meaningful name
89+
* can be inferred, there is no result.
90+
*/
91+
string getName() {
92+
result = getId().getName()
93+
or
94+
not exists(getId()) and
95+
(
96+
exists(VarDef vd | this = vd.getSource() | result = vd.getTarget().(VarRef).getName())
97+
or
98+
exists(Property p |
99+
this = p.getInit() and
100+
result = p.getName()
101+
)
102+
or
103+
exists(AssignExpr assign, PropAccess prop |
104+
this = assign.getRhs().getUnderlyingValue() and
105+
prop = assign.getLhs() and
106+
result = prop.getPropertyName()
107+
)
108+
or
109+
exists(ClassOrInterface c | this = c.getMember(result).getInit())
110+
)
111+
}
85112

86113
/** Gets the variable holding this function. */
87114
Variable getVariable() { result = getId().getVariable() }
@@ -274,8 +301,8 @@ class Function extends @function, Parameterized, TypeParameterized, StmtContaine
274301
*/
275302
private string inferNameFromVarDef() {
276303
// in ambiguous cases like `var f = function g() {}`, prefer `g` to `f`
277-
if exists(getName())
278-
then result = "function " + getName()
304+
if exists(getId())
305+
then result = "function " + getId().getName()
279306
else
280307
exists(VarDef vd | this = vd.getSource() |
281308
result = "function " + vd.getTarget().(VarRef).getName()

javascript/ql/src/semmle/javascript/dataflow/Nodes.qll

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -340,12 +340,12 @@ class FunctionNode extends DataFlow::ValueNode, DataFlow::SourceNode {
340340
int getNumParameter() { result = count(astNode.getAParameter()) }
341341

342342
/** Gets the last parameter of this function. */
343-
ParameterNode getLastParameter() { result = getParameter(getNumParameter()-1) }
343+
ParameterNode getLastParameter() { result = getParameter(getNumParameter() - 1) }
344344

345345
/** Holds if the last parameter of this function is a rest parameter. */
346346
predicate hasRestParameter() { astNode.hasRestParameter() }
347347

348-
/** Gets the name of this function, if it has one. */
348+
/** Gets the unqualified name of this function, if it has one or one can be determined from the context. */
349349
string getName() { result = astNode.getName() }
350350

351351
/** Gets a data flow node corresponding to a return value of this function. */
@@ -576,7 +576,7 @@ class ClassNode extends DataFlow::SourceNode {
576576
ClassNode() { this = impl }
577577

578578
/**
579-
* Gets the name of the class, if it has one.
579+
* Gets the unqualified name of the class, if it has one or one can be determined from the context.
580580
*/
581581
string getName() { result = impl.getName() }
582582

@@ -651,16 +651,12 @@ class ClassNode extends DataFlow::SourceNode {
651651
/**
652652
* Gets a direct super class of this class.
653653
*/
654-
ClassNode getADirectSuperClass() {
655-
result.getAClassReference().flowsTo(getASuperClassNode())
656-
}
654+
ClassNode getADirectSuperClass() { result.getAClassReference().flowsTo(getASuperClassNode()) }
657655

658656
/**
659657
* Gets a direct subclass of this class.
660658
*/
661-
final ClassNode getADirectSubClass() {
662-
this = result.getADirectSuperClass()
663-
}
659+
final ClassNode getADirectSubClass() { this = result.getADirectSuperClass() }
664660

665661
/**
666662
* Gets the receiver of an instance member or constructor of this class.
@@ -674,16 +670,12 @@ class ClassNode extends DataFlow::SourceNode {
674670
/**
675671
* Gets the abstract value representing the class itself.
676672
*/
677-
AbstractValue getAbstractClassValue() {
678-
result = this.(AnalyzedNode).getAValue()
679-
}
673+
AbstractValue getAbstractClassValue() { result = this.(AnalyzedNode).getAValue() }
680674

681675
/**
682676
* Gets the abstract value representing an instance of this class.
683677
*/
684-
AbstractValue getAbstractInstanceValue() {
685-
result = AbstractInstance::of(getAstNode())
686-
}
678+
AbstractValue getAbstractInstanceValue() { result = AbstractInstance::of(getAstNode()) }
687679

688680
/**
689681
* Gets a dataflow node that refers to this class object.
@@ -692,9 +684,7 @@ class ClassNode extends DataFlow::SourceNode {
692684
t.start() and
693685
result.(AnalyzedNode).getAValue() = getAbstractClassValue()
694686
or
695-
exists(DataFlow::TypeTracker t2 |
696-
result = getAClassReference(t2).track(t2, t)
697-
)
687+
exists(DataFlow::TypeTracker t2 | result = getAClassReference(t2).track(t2, t))
698688
}
699689

700690
/**
@@ -725,9 +715,7 @@ class ClassNode extends DataFlow::SourceNode {
725715

726716
pragma[noinline]
727717
private DataFlow::SourceNode getAnInstanceReferenceAux(DataFlow::TypeTracker t) {
728-
exists(DataFlow::TypeTracker t2 |
729-
result = getAnInstanceReference(t2).track(t2, t)
730-
)
718+
exists(DataFlow::TypeTracker t2 | result = getAnInstanceReference(t2).track(t2, t))
731719
}
732720

733721
/**
@@ -846,11 +834,12 @@ module ClassNode {
846834
*/
847835
class FunctionStyleClass extends Range, DataFlow::ValueNode {
848836
override Function astNode;
837+
849838
AbstractFunction function;
850839

851840
FunctionStyleClass() {
852841
function.getFunction() = astNode and
853-
exists (DataFlow::PropRef read |
842+
exists(DataFlow::PropRef read |
854843
read.getPropertyName() = "prototype" and
855844
read.getBase().analyze().getAValue() = function
856845
)
@@ -902,15 +891,13 @@ module ClassNode {
902891

903892
override FunctionNode getStaticMethod(string name) { result = getAPropertySource(name) }
904893

905-
override FunctionNode getAStaticMethod() {
906-
result = getAPropertySource()
907-
}
894+
override FunctionNode getAStaticMethod() { result = getAPropertySource() }
908895

909896
/**
910897
* Gets a reference to the prototype of this class.
911898
*/
912899
DataFlow::SourceNode getAPrototypeReference() {
913-
exists (DataFlow::SourceNode base | base.analyze().getAValue() = function |
900+
exists(DataFlow::SourceNode base | base.analyze().getAValue() = function |
914901
result = base.getAPropertyRead("prototype")
915902
or
916903
result = base.getAPropertySource("prototype")

javascript/ql/test/library-tests/Classes/tests.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ test_ClassDefinition_getName
3232
| points.js:1:1:18:1 | class P ... ;\\n }\\n} | Point |
3333
| points.js:20:1:33:1 | class C ... ;\\n }\\n} | ColouredPoint |
3434
| staticConstructor.js:1:1:3:1 | class M ... r"; }\\n} | MyClass |
35+
| tst.js:1:9:4:1 | class { ... */ }\\n} | A |
3536
| tst.js:6:1:8:1 | class B ... t); }\\n} | B |
3637
| tst.js:11:1:14:1 | class C ... () {}\\n} | C |
3738
test_MethodDefinitions
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
1+
getName
2+
| tst.js:16:1:19:1 | class C ... ss C"\\n} | C |
3+
| tst.js:21:2:23:1 | class D ... lass"\\n} | D |
4+
| tst.js:25:11:25:18 | class {} | E |
5+
| tst.js:26:11:29:1 | class G ... ss G"\\n} | G |
6+
| tst.js:34:9:34:16 | class {} | Foo |
7+
#select
18
| tst.js:16:1:19:1 | class C ... ss C"\\n} | class C |
29
| tst.js:21:2:23:1 | class D ... lass"\\n} | class D |
310
| tst.js:22:12:22:18 | class{} | anonymous class |
411
| tst.js:25:11:25:18 | class {} | class E |
512
| tst.js:26:11:29:1 | class G ... ss G"\\n} | class G |
13+
| tst.js:34:9:34:16 | class {} | anonymous class |
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import javascript
22

3+
query string getName(ClassDefinition c) { result = c.getName() }
4+
35
from ClassDefinition c
46
select c, c.describe()

javascript/ql/test/library-tests/ppNames/ppFnName.expected

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,26 @@
1+
getName
2+
| tst.js:1:1:1:15 | function f() {} | f |
3+
| tst.js:2:2:2:16 | function g() {} | g |
4+
| tst.js:4:9:4:22 | function () {} | h |
5+
| tst.js:5:9:5:14 | x => x | k |
6+
| tst.js:6:9:6:23 | function n() {} | n |
7+
| tst.js:9:6:9:18 | function() {} | p |
8+
| tst.js:10:6:10:20 | function f() {} | f |
9+
| tst.js:11:8:11:12 | () {} | x |
10+
| tst.js:12:8:12:13 | (v) {} | x |
11+
| tst.js:13:4:13:8 | () {} | m |
12+
| tst.js:17:14:17:18 | () {} | constructor |
13+
| tst.js:18:4:18:8 | () {} | n |
14+
| tst.js:22:17:22:16 | () {} | constructor |
15+
| tst.js:22:21:22:20 | (...arg ... rgs); } | constructor |
16+
| tst.js:25:17:25:16 | () {} | constructor |
17+
| tst.js:26:19:26:18 | () {} | constructor |
18+
| tst.js:27:8:27:12 | () {} | y |
19+
| tst.js:28:8:28:13 | (v) {} | y |
20+
| tst.js:31:9:31:21 | function() {} | foo |
21+
| tst.js:32:12:32:24 | function() {} | Hey |
22+
| tst.js:34:15:34:14 | () {} | constructor |
23+
#select
124
| tst.js:1:1:1:15 | function f() {} | function f |
225
| tst.js:2:2:2:16 | function g() {} | function g |
326
| tst.js:3:2:3:14 | function() {} | anonymous function |
@@ -17,3 +40,6 @@
1740
| tst.js:26:19:26:18 | () {} | default constructor of class G |
1841
| tst.js:27:8:27:12 | () {} | getter method for property y of class G |
1942
| tst.js:28:8:28:13 | (v) {} | setter method for property y of class G |
43+
| tst.js:31:9:31:21 | function() {} | anonymous function |
44+
| tst.js:32:12:32:24 | function() {} | anonymous function |
45+
| tst.js:34:15:34:14 | () {} | default constructor of anonymous class |
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import javascript
22

3+
query string getName(Function f) { result = f.getName() }
4+
35
from Function f
46
select f, f.describe()

0 commit comments

Comments
 (0)