Skip to content

Commit 37cd517

Browse files
max-leuthaeusermax-leuthaeuser
committed
Set PARAMETER_INDEX for all MethodParameterIn nodes
For: ShiftLeftSecurity/codescience#8203
1 parent 44946f0 commit 37cd517

File tree

6 files changed

+67
-65
lines changed

6 files changed

+67
-65
lines changed

‎src/main/scala/io/shiftleft/js2cpg/astcreation/AstCreator.scala‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,10 @@ class AstCreator(diffGraph: DiffGraphBuilder, source: JsSource, usedIdentNodes:
144144

145145
private def createIdentifierNode(name: String, lineAndColumnProvider: Node): NewIdentifier = {
146146
val dynamicInstanceTypeOption = name match {
147-
case "this" => dynamicInstanceTypeStack.headOption
148-
case "console" => Some(Defines.Console)
149-
case "Math" => Some(Defines.Math)
150-
case _ => None
147+
case Defines.This => dynamicInstanceTypeStack.headOption
148+
case "console" => Some(Defines.Console)
149+
case "Math" => Some(Defines.Math)
150+
case _ => None
151151
}
152152
astNodeBuilder.createIdentifierNode(name, lineAndColumnProvider, dynamicInstanceTypeOption)
153153
}
@@ -277,16 +277,16 @@ class AstCreator(diffGraph: DiffGraphBuilder, source: JsSource, usedIdentNodes:
277277
}
278278
scope.pushNewMethodScope(methodFullName, methodName, blockId, capturingRefId)
279279

280-
val parameterOrderTracker = new OrderTracker(0)
280+
val parameterIndexTracker = new OrderTracker(0)
281281
// We always create an instance parameter because in JS every function could get called with an instance.
282-
astNodeBuilder.createParameterInNode("this", "this", methodId, functionNode, parameterOrderTracker)
282+
astNodeBuilder.createParameterInNode(Defines.This, Defines.This, methodId, functionNode, parameterIndexTracker)
283283
functionNode.getParameters.forEach { parameter =>
284284
astNodeBuilder.createParameterInNode(
285285
parameter.getName,
286286
source.getString(parameter),
287287
methodId,
288288
parameter,
289-
parameterOrderTracker
289+
parameterIndexTracker
290290
)
291291
}
292292

@@ -566,7 +566,7 @@ class AstCreator(diffGraph: DiffGraphBuilder, source: JsSource, usedIdentNodes:
566566
} else {
567567
val (functionBaseId, functionPropertyId, receiverId, baseId) = callNode.getFunction match {
568568
case functionAccessNode: AccessNode =>
569-
// "this" argument is coming from source.
569+
// Defines.This argument is coming from source.
570570
functionAccessNode.getBase match {
571571
case baseIdentNode: IdentNode =>
572572
// TODO The check for IdentNode is too restrictive.
@@ -617,8 +617,8 @@ class AstCreator(diffGraph: DiffGraphBuilder, source: JsSource, usedIdentNodes:
617617
val receiverId = callNode.getFunction.accept(this)
618618

619619
// We need to create a synthetic this argument.
620-
val thisId = createIdentifierNode("this", callNode)
621-
scope.addVariableReference("this", thisId, Defines.Any, EvaluationStrategies.BY_REFERENCE)
620+
val thisId = createIdentifierNode(Defines.This, callNode)
621+
scope.addVariableReference(Defines.This, thisId, Defines.Any, EvaluationStrategies.BY_REFERENCE)
622622

623623
(receiverId, None, receiverId, thisId)
624624
}
@@ -735,7 +735,7 @@ class AstCreator(diffGraph: DiffGraphBuilder, source: JsSource, usedIdentNodes:
735735
astNodeBuilder.lineAndColumn(forNode)
736736
)
737737

738-
val thisId = createIdentifierNode("this", forNode)
738+
val thisId = createIdentifierNode(Defines.This, forNode)
739739

740740
val indexCallId = astNodeBuilder.createCallNode(
741741
"Object.keys(" + collectionName + ")[Symbol.iterator]",

‎src/main/scala/io/shiftleft/js2cpg/astcreation/AstNodeBuilder.scala‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class AstNodeBuilder(
4848
code: String,
4949
methodNode: NewMethod,
5050
lineAndColumnProvider: Node,
51-
orderTracker: OrderTracker
51+
parameterIndexTracker: OrderTracker
5252
): NewMethodParameterIn = {
5353
val lineColumn = lineAndColumn(lineAndColumnProvider)
5454
val line = lineColumn.line
@@ -59,11 +59,11 @@ class AstNodeBuilder(
5959
.evaluationStrategy(EvaluationStrategies.BY_VALUE)
6060
.lineNumber(line)
6161
.columnNumber(column)
62-
.order(orderTracker.order)
62+
.index(parameterIndexTracker.order)
6363
.typeFullName(Defines.Any)
6464

6565
diffGraph.addNode(param)
66-
orderTracker.inc()
66+
parameterIndexTracker.inc()
6767
astEdgeBuilder.addAstEdge(param, methodNode)
6868
scope.addVariable(name, param, Defines.Any, MethodScope)
6969
param

‎src/main/scala/io/shiftleft/js2cpg/passes/Defines.scala‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,6 @@ object Defines {
1515

1616
val JsTypes: List[String] =
1717
List(Any, Number, String, Boolean, Null, Math, Console)
18+
19+
val This: String = "this"
1820
}

‎src/test/scala/io/shiftleft/js2cpg/passes/CfgCreationPassTest.scala‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ class CfgCreationPassTest extends AnyWordSpec with Matchers {
1818
succOf(":program") shouldBe expected(("x", AlwaysEdge))
1919
succOf("x") shouldBe expected(("class Foo", AlwaysEdge))
2020
succOf("class Foo") shouldBe expected(("bar", AlwaysEdge))
21-
succOf("bar") shouldBe expected(("this", AlwaysEdge))
22-
succOf("this") shouldBe expected(("bar()", AlwaysEdge))
21+
succOf("bar") shouldBe expected((Defines.This, AlwaysEdge))
22+
succOf(Defines.This) shouldBe expected(("bar()", AlwaysEdge))
2323
succOf("bar()") shouldBe expected(("class Foo , bar()", AlwaysEdge))
2424
succOf("class Foo , bar()") shouldBe expected(("x = class Foo , bar()", AlwaysEdge))
2525
succOf("x = class Foo , bar()") shouldBe expected(("RET", AlwaysEdge))
@@ -65,8 +65,8 @@ class CfgCreationPassTest extends AnyWordSpec with Matchers {
6565
"have correct structure for untagged runtime node in call" in {
6666
new CfgFixture(s"foo(`Hello $${world}!`)") {
6767
succOf(":program") shouldBe expected(("foo", AlwaysEdge))
68-
succOf("foo") shouldBe expected(("this", AlwaysEdge))
69-
succOf("this") shouldBe expected(("\"Hello \"", AlwaysEdge))
68+
succOf("foo") shouldBe expected((Defines.This, AlwaysEdge))
69+
succOf(Defines.This) shouldBe expected(("\"Hello \"", AlwaysEdge))
7070
succOf("\"Hello \"") shouldBe expected(("world", AlwaysEdge))
7171
succOf("world") shouldBe expected(("\"!\"", AlwaysEdge))
7272
succOf("\"!\"") shouldBe expected(("__Runtime.TO_STRING(\"Hello \",world,\"!\")", AlwaysEdge))
@@ -107,13 +107,13 @@ class CfgCreationPassTest extends AnyWordSpec with Matchers {
107107
"be correct for try" in {
108108
new CfgFixture("try { open() } catch(err) { handle() } finally { close() }".stripMargin) {
109109
succOf(":program") shouldBe expected(("open", AlwaysEdge))
110-
succOf("open") shouldBe expected(("this", AlwaysEdge))
111-
succOf("this") shouldBe expected(("open()", AlwaysEdge))
110+
succOf("open") shouldBe expected((Defines.This, AlwaysEdge))
111+
succOf(Defines.This) shouldBe expected(("open()", AlwaysEdge))
112112
succOf("open()") shouldBe expected(("handle", AlwaysEdge), ("close", AlwaysEdge))
113113
succOf("handle()") shouldBe expected(("{ handle() }", AlwaysEdge))
114114
succOf("{ handle() }") shouldBe expected(("close", AlwaysEdge))
115-
succOf("close") shouldBe expected(("this", 2, AlwaysEdge))
116-
succOf("this", 2) shouldBe expected(("close()", AlwaysEdge))
115+
succOf("close") shouldBe expected((Defines.This, 2, AlwaysEdge))
116+
succOf(Defines.This, 2) shouldBe expected(("close()", AlwaysEdge))
117117
succOf("close()") shouldBe expected(("RET", AlwaysEdge))
118118
}
119119
}
@@ -238,8 +238,8 @@ class CfgCreationPassTest extends AnyWordSpec with Matchers {
238238
"be correct for call expression" in {
239239
new CfgFixture("foo(a + 1, b);") {
240240
succOf(":program") shouldBe expected(("foo", AlwaysEdge))
241-
succOf("foo") shouldBe expected(("this", AlwaysEdge))
242-
succOf("this") shouldBe expected(("a", AlwaysEdge))
241+
succOf("foo") shouldBe expected((Defines.This, AlwaysEdge))
242+
succOf(Defines.This) shouldBe expected(("a", AlwaysEdge))
243243
succOf("a") shouldBe expected(("1", AlwaysEdge))
244244
succOf("1") shouldBe expected(("a + 1", AlwaysEdge))
245245
succOf("a + 1") shouldBe expected(("b", AlwaysEdge))
@@ -702,11 +702,11 @@ class CfgCreationPassTest extends AnyWordSpec with Matchers {
702702
new CfgFixture("const { a, b } = test() ? foo() : bar();") {
703703
succOf(":program") shouldBe expected(("_tmp_0", AlwaysEdge))
704704
succOf("_tmp_0") shouldBe expected(("test", AlwaysEdge))
705-
succOf("test") shouldBe expected(("this", AlwaysEdge))
706-
succOf("this") shouldBe expected(("test()", AlwaysEdge))
705+
succOf("test") shouldBe expected((Defines.This, AlwaysEdge))
706+
succOf(Defines.This) shouldBe expected(("test()", AlwaysEdge))
707707
succOf("test()") shouldBe expected(("foo", TrueEdge), ("bar", FalseEdge))
708-
succOf("foo") shouldBe expected(("this", 1, AlwaysEdge))
709-
succOf("this", 1) shouldBe expected(("foo()", AlwaysEdge))
708+
succOf("foo") shouldBe expected((Defines.This, 1, AlwaysEdge))
709+
succOf(Defines.This, 1) shouldBe expected(("foo()", AlwaysEdge))
710710
succOf("bar()") shouldBe expected(("test() ? foo() : bar()", AlwaysEdge))
711711
succOf("foo()") shouldBe expected(("test() ? foo() : bar()", AlwaysEdge))
712712
succOf("test() ? foo() : bar()") shouldBe expected(("_tmp_0 = test() ? foo() : bar()", AlwaysEdge))
@@ -1039,8 +1039,8 @@ class CfgCreationPassTest extends AnyWordSpec with Matchers {
10391039
"be correct for await/async" in {
10401040
new CfgFixture("async function(foo) { await foo() }") {
10411041
succOf("anonymous", NodeTypes.METHOD) shouldBe expected(("foo", AlwaysEdge))
1042-
succOf("foo") shouldBe expected(("this", AlwaysEdge))
1043-
succOf("this", NodeTypes.IDENTIFIER) shouldBe expected(("foo()", AlwaysEdge))
1042+
succOf("foo") shouldBe expected((Defines.This, AlwaysEdge))
1043+
succOf(Defines.This, NodeTypes.IDENTIFIER) shouldBe expected(("foo()", AlwaysEdge))
10441044
succOf("foo()") shouldBe expected(("await foo()", AlwaysEdge))
10451045
succOf("await foo()") shouldBe expected(("RET", AlwaysEdge))
10461046
}
@@ -1119,8 +1119,8 @@ class CfgCreationPassTest extends AnyWordSpec with Matchers {
11191119
|}
11201120
|""".stripMargin) {
11211121
succOf("foo", NodeTypes.METHOD) shouldBe expected(("bar", AlwaysEdge))
1122-
succOf("bar") shouldBe expected(("this", AlwaysEdge))
1123-
succOf("this", NodeTypes.IDENTIFIER) shouldBe expected(("bar()", AlwaysEdge))
1122+
succOf("bar") shouldBe expected((Defines.This, AlwaysEdge))
1123+
succOf(Defines.This, NodeTypes.IDENTIFIER) shouldBe expected(("bar()", AlwaysEdge))
11241124
succOf("bar()") shouldBe expected(("RET", 2, AlwaysEdge))
11251125
}
11261126
}
@@ -1353,8 +1353,8 @@ class CfgCreationPassTest extends AnyWordSpec with Matchers {
13531353
succOf("Symbol") shouldBe expected(("iterator", AlwaysEdge))
13541354
succOf("iterator") shouldBe expected(("Symbol.iterator", AlwaysEdge))
13551355
succOf("Symbol.iterator") shouldBe expected(("Object.keys(arr)[Symbol.iterator]", AlwaysEdge))
1356-
succOf("Object.keys(arr)[Symbol.iterator]") shouldBe expected(("this", AlwaysEdge))
1357-
succOf("this") shouldBe expected(("Object.keys(arr)[Symbol.iterator]()", AlwaysEdge))
1356+
succOf("Object.keys(arr)[Symbol.iterator]") shouldBe expected((Defines.This, AlwaysEdge))
1357+
succOf(Defines.This) shouldBe expected(("Object.keys(arr)[Symbol.iterator]()", AlwaysEdge))
13581358
succOf("Object.keys(arr)[Symbol.iterator]()") shouldBe expected(
13591359
("_iterator_0 = Object.keys(arr)[Symbol.iterator]()", AlwaysEdge)
13601360
)
@@ -1386,8 +1386,8 @@ class CfgCreationPassTest extends AnyWordSpec with Matchers {
13861386
succOf("value") shouldBe expected(("_result_0.value", AlwaysEdge))
13871387
succOf("_result_0.value") shouldBe expected(("i = _result_0.value", AlwaysEdge))
13881388
succOf("i = _result_0.value") shouldBe expected(("foo", AlwaysEdge))
1389-
succOf("foo") shouldBe expected(("this", 1, AlwaysEdge))
1390-
succOf("this", 1) shouldBe expected(("i", 2, AlwaysEdge))
1389+
succOf("foo") shouldBe expected((Defines.This, 1, AlwaysEdge))
1390+
succOf(Defines.This, 1) shouldBe expected(("i", 2, AlwaysEdge))
13911391
succOf("i", 2) shouldBe expected(("foo(i)", AlwaysEdge))
13921392
succOf("foo(i)") shouldBe expected(("{ foo(i) }", AlwaysEdge))
13931393
succOf("{ foo(i) }") shouldBe expected(("_result_0", 1, AlwaysEdge))

‎src/test/scala/io/shiftleft/js2cpg/passes/MixedAstCreationPassTest.scala‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -750,10 +750,10 @@ class MixedAstCreationPassTest extends AbstractPassTest {
750750
val List(fooMethod) = program.astChildren.isMethod.nameExact("foo").l
751751
val List(a) = fooMethod.parameter.nameExact("param1_0").l
752752
a.code shouldBe "{a}"
753-
a.order shouldBe 1
753+
a.index shouldBe 1
754754
val List(b) = fooMethod.parameter.nameExact("b").l
755755
b.code shouldBe "b"
756-
b.order shouldBe 2
756+
b.index shouldBe 2
757757
}
758758

759759
"have correct structure for object destruction assignment in call argument" in AstFixture("foo({a, b} = x);") {
@@ -1070,7 +1070,7 @@ class MixedAstCreationPassTest extends AbstractPassTest {
10701070
val List(foo) = cpg.method.nameExact("foo").l
10711071

10721072
val List(paramA) = foo.parameter.nameExact("a").l
1073-
paramA.order shouldBe 1
1073+
paramA.index shouldBe 1
10741074

10751075
val List(block) = foo.astChildren.isBlock.l
10761076
val List(assignment) = block.astChildren.isCall.l
@@ -1089,10 +1089,10 @@ class MixedAstCreationPassTest extends AbstractPassTest {
10891089
) { cpg =>
10901090
val List(foo) = cpg.method.nameExact("foo").l
10911091
val List(paramA) = foo.parameter.nameExact("a").l
1092-
paramA.order shouldBe 1
1092+
paramA.index shouldBe 1
10931093

10941094
val List(paramB) = foo.parameter.nameExact("b").l
1095-
paramB.order shouldBe 2
1095+
paramB.index shouldBe 2
10961096

10971097
val List(block) = foo.astChildren.isBlock.l
10981098

@@ -1122,9 +1122,9 @@ class MixedAstCreationPassTest extends AbstractPassTest {
11221122
cpg =>
11231123
val List(foo) = cpg.method.nameExact("foo").l
11241124
val List(paramA) = foo.parameter.nameExact("a").l
1125-
paramA.order shouldBe 1
1125+
paramA.index shouldBe 1
11261126
val List(paramB) = foo.parameter.nameExact("b").l
1127-
paramB.order shouldBe 2
1127+
paramB.index shouldBe 2
11281128

11291129
val List(block) = foo.astChildren.isBlock.l
11301130
val List(assignmentB) = block.astChildren.isCall.codeExact("b = b === void 0 ? 1 : b").l
@@ -1143,11 +1143,11 @@ class MixedAstCreationPassTest extends AbstractPassTest {
11431143
) { cpg =>
11441144
val List(foo) = cpg.method.nameExact("foo").l
11451145
val List(paramA) = foo.parameter.nameExact("a").l
1146-
paramA.order shouldBe 1
1146+
paramA.index shouldBe 1
11471147
val List(paramB) = foo.parameter.nameExact("b").l
1148-
paramB.order shouldBe 2
1148+
paramB.index shouldBe 2
11491149
val List(paramC) = foo.parameter.nameExact("c").l
1150-
paramC.order shouldBe 3
1150+
paramC.index shouldBe 3
11511151

11521152
val List(block) = foo.astChildren.isBlock.l
11531153
val List(assignmentB) = block.astChildren.isCall.codeExact("b = b === void 0 ? 1 : b").l

‎src/test/scala/io/shiftleft/js2cpg/passes/SimpleAstCreationPassTest.scala‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -282,11 +282,11 @@ class SimpleAstCreationPassTest extends AbstractPassTest {
282282
val List(lambdaBlock) = lambda.astChildren.isBlock.l
283283

284284
val List(param1, param2) = lambda.parameter.l
285-
param1.order shouldBe 0
286-
param1.name shouldBe "this"
287-
param1.code shouldBe "this"
285+
param1.index shouldBe 0
286+
param1.name shouldBe Defines.This
287+
param1.code shouldBe Defines.This
288288

289-
param2.order shouldBe 1
289+
param2.index shouldBe 1
290290
param2.name shouldBe "param1_0"
291291
param2.code shouldBe "{param}"
292292

@@ -337,7 +337,7 @@ class SimpleAstCreationPassTest extends AbstractPassTest {
337337
receiver.name shouldBe "foo"
338338
receiver.argumentIndex shouldBe -1
339339

340-
val List(argumentThis) = fooCall.astChildren.isIdentifier.nameExact("this").l
340+
val List(argumentThis) = fooCall.astChildren.isIdentifier.nameExact(Defines.This).l
341341
argumentThis.argumentIndex shouldBe 0
342342

343343
val List(argument1) = fooCall.astChildren.isIdentifier.nameExact("x").l
@@ -452,19 +452,19 @@ class SimpleAstCreationPassTest extends AbstractPassTest {
452452
"have correct structure for empty method" in AstFixture("function method(x) {}") { cpg =>
453453
val List(method) = cpg.method.nameExact("method").l
454454
method.astChildren.isBlock.size shouldBe 1
455-
method.parameter.order(0).nameExact("this").typeFullName(Defines.Any).size shouldBe 1
456-
method.parameter.order(1).nameExact("x").typeFullName(Defines.Any).size shouldBe 1
455+
method.parameter.index(0).nameExact(Defines.This).typeFullName(Defines.Any).size shouldBe 1
456+
method.parameter.index(1).nameExact("x").typeFullName(Defines.Any).size shouldBe 1
457457
}
458458

459459
"have correct structure for decl assignment" in AstFixture("function foo(x) { var local = 1; }") { cpg =>
460460
val List(method) = cpg.method.nameExact("foo").l
461461
val List(block) = method.astChildren.isBlock.l
462462

463463
val List(t, x) = method.parameter.l
464-
t.order shouldBe 0
465-
t.name shouldBe "this"
464+
t.index shouldBe 0
465+
t.name shouldBe Defines.This
466466
t.typeFullName shouldBe Defines.Any
467-
x.order shouldBe 1
467+
x.index shouldBe 1
468468
x.name shouldBe "x"
469469
x.typeFullName shouldBe Defines.Any
470470

@@ -483,10 +483,10 @@ class SimpleAstCreationPassTest extends AbstractPassTest {
483483
val List(block) = method.astChildren.isBlock.l
484484

485485
val List(t, x) = method.parameter.l
486-
t.order shouldBe 0
487-
t.name shouldBe "this"
486+
t.index shouldBe 0
487+
t.name shouldBe Defines.This
488488
t.typeFullName shouldBe Defines.Any
489-
x.order shouldBe 1
489+
x.index shouldBe 1
490490
x.name shouldBe "x"
491491
x.typeFullName shouldBe Defines.Any
492492

@@ -506,13 +506,13 @@ class SimpleAstCreationPassTest extends AbstractPassTest {
506506
val List(block) = method.astChildren.isBlock.l
507507

508508
val List(t, x, y) = method.parameter.l
509-
t.order shouldBe 0
510-
t.name shouldBe "this"
509+
t.index shouldBe 0
510+
t.name shouldBe Defines.This
511511
t.typeFullName shouldBe Defines.Any
512-
x.order shouldBe 1
512+
x.index shouldBe 1
513513
x.name shouldBe "x"
514514
x.typeFullName shouldBe Defines.Any
515-
y.order shouldBe 2
515+
y.index shouldBe 2
516516
y.name shouldBe "y"
517517
y.typeFullName shouldBe Defines.Any
518518

@@ -680,10 +680,10 @@ class SimpleAstCreationPassTest extends AbstractPassTest {
680680
|}""".stripMargin) { cpg =>
681681
val List(method) = cpg.method.nameExact("method").l
682682

683-
val List(parameterInX) = method.parameter.order(1).l
683+
val List(parameterInX) = method.parameter.index(1).l
684684
parameterInX.name shouldBe "x"
685685

686-
val List(parameterInY) = method.parameter.order(2).l
686+
val List(parameterInY) = method.parameter.index(2).l
687687
parameterInY.name shouldBe "y"
688688

689689
val List(methodBlock) = method.astChildren.isBlock.l

0 commit comments

Comments
 (0)