Skip to content

Commit d63c849

Browse files
max-leuthaeusermax-leuthaeuser
committed
fix for review comment
1 parent b88715b commit d63c849

File tree

7 files changed

+182
-45
lines changed

7 files changed

+182
-45
lines changed

‎joern-cli/frontends/swiftsrc2cpg/src/main/scala/io/joern/swiftsrc2cpg/astcreation/AstCreator.scala‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,21 +36,11 @@ class AstCreator(
3636

3737
protected val logger: Logger = LoggerFactory.getLogger(classOf[AstCreator])
3838

39-
protected val scope = new SwiftVariableScopeManager()
40-
protected val fullnameProvider = new FullnameProvider(typeMap)
41-
protected val methodAstParentStack = new Stack[NewNode]()
42-
protected val typeRefIdStack = new Stack[NewTypeRef]
43-
protected val localAstParentStack = new Stack[NewBlock]()
44-
45-
case class InstanceTypeStackElement(name: String, members: mutable.HashSet[String])
46-
protected val dynamicInstanceTypeStack = new Stack[InstanceTypeStackElement]
47-
48-
protected def addMemberToDynamicInstanceTypeStack(typeDeclFullName: String, memberName: String): Unit = {
49-
dynamicInstanceTypeStack.find(_.name == typeDeclFullName).foreach { elem =>
50-
elem.members.addOne(memberName)
51-
}
52-
}
53-
39+
protected val scope = new SwiftVariableScopeManager()
40+
protected val fullnameProvider = new FullnameProvider(typeMap)
41+
protected val methodAstParentStack = new Stack[NewNode]()
42+
protected val typeRefIdStack = new Stack[NewTypeRef]
43+
protected val localAstParentStack = new Stack[NewBlock]()
5444
protected val scopeLocalUniqueNames = mutable.HashMap.empty[String, Int]
5545

5646
protected lazy val definedSymbols: Map[String, String] = {
@@ -83,6 +73,7 @@ class AstCreator(
8373
val fakeGlobalMethod =
8474
methodNode(ast, name, name, fullName, None, path, Option(NodeTypes.TYPE_DECL), Option(fullName))
8575
methodAstParentStack.push(fakeGlobalMethod)
76+
scope.pushNewTypeDeclScope(fullName)
8677
scope.pushNewMethodScope(fullName, name, fakeGlobalMethod, None)
8778
val sourceFileAst = astForNode(ast)
8879
val methodReturn = methodReturnNode(ast, Defines.Any)

‎joern-cli/frontends/swiftsrc2cpg/src/main/scala/io/joern/swiftsrc2cpg/astcreation/AstCreatorHelper.scala‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -177,14 +177,10 @@ trait AstCreatorHelper(implicit withSchemaValidation: ValidationMode) { this: As
177177
identNode.typeFullName = tpe
178178
scope.addVariableReference(identifierName, identNode, tpe, EvaluationStrategies.BY_REFERENCE)
179179
Ast(identNode)
180-
} else if (
181-
dynamicInstanceTypeStack.headOption.exists { case InstanceTypeStackElement(tpe, members) =>
182-
members.contains(identifierName)
183-
}
184-
) {
180+
} else if (scope.variableIsInTypeDeclScope(identifierName)) {
185181
// 2) we found it as member of the surrounding type decl:
186182
// (Swift does not allow to access any member / function of the outer class instance)
187-
val tpe = dynamicInstanceTypeStack.head.name
183+
val tpe = typeForSelfExpression()
188184
val selfNode = identifierNode(node, "self", "self", tpe)
189185
scope.addVariableReference("self", selfNode, selfNode.typeFullName, EvaluationStrategies.BY_REFERENCE)
190186
fieldAccessAst(node, node, Ast(selfNode), s"self.$identifierName", identifierName, tpe)

‎joern-cli/frontends/swiftsrc2cpg/src/main/scala/io/joern/swiftsrc2cpg/astcreation/AstForDeclSyntaxCreator.scala‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ trait AstForDeclSyntaxCreator(implicit withSchemaValidation: ValidationMode) {
233233
val tpeFromTypeMap = fullnameProvider.typeFullname(c)
234234
val typeFullName = tpeFromTypeMap.getOrElse(typeNameForDeclSyntax(d))
235235
val memberNode_ = memberNode(c, cCode, cCode, typeFullName)
236-
addMemberToDynamicInstanceTypeStack(typeDeclNode.fullName, cCode)
236+
scope.addVariable(cCode, memberNode_, typeFullName, VariableScopeManager.ScopeType.TypeDeclScope)
237237
diffGraph.addEdge(typeDeclNode, memberNode_, EdgeTypes.AST)
238238
}
239239
ast
@@ -244,7 +244,7 @@ trait AstForDeclSyntaxCreator(implicit withSchemaValidation: ValidationMode) {
244244
val tpeFromTypeMap = fullnameProvider.typeFullname(c)
245245
val typeFullName = tpeFromTypeMap.getOrElse(typeNameForDeclSyntax(d))
246246
val memberNode_ = memberNode(c, cCode, cCode, typeFullName)
247-
addMemberToDynamicInstanceTypeStack(typeDeclNode.fullName, cCode)
247+
scope.addVariable(cCode, memberNode_, typeFullName, VariableScopeManager.ScopeType.TypeDeclScope)
248248
diffGraph.addEdge(typeDeclNode, memberNode_, EdgeTypes.AST)
249249
}
250250
ast
@@ -331,8 +331,8 @@ trait AstForDeclSyntaxCreator(implicit withSchemaValidation: ValidationMode) {
331331
}
332332

333333
methodAstParentStack.push(typeDeclNode_)
334-
dynamicInstanceTypeStack.push(InstanceTypeStackElement(typeFullName, mutable.HashSet.empty))
335334
typeRefIdStack.push(typeRefNode_)
335+
scope.pushNewTypeDeclScope(typeFullName)
336336
scope.pushNewMethodScope(typeFullName, typeName, typeDeclNode_, None)
337337

338338
val allClassMembers = declMembers(node, withConstructor = false).toList
@@ -350,9 +350,9 @@ trait AstForDeclSyntaxCreator(implicit withSchemaValidation: ValidationMode) {
350350
val staticMemberInits = allClassMembers.filter(isStaticMember)
351351

352352
methodAstParentStack.pop()
353-
dynamicInstanceTypeStack.pop()
354353
typeRefIdStack.pop()
355354
scope.popScope()
355+
scope.popScope()
356356

357357
if (staticMemberInits.nonEmpty) {
358358
val init = staticInitMethodAstAndBlock(
@@ -471,8 +471,8 @@ trait AstForDeclSyntaxCreator(implicit withSchemaValidation: ValidationMode) {
471471
}
472472

473473
methodAstParentStack.push(typeDeclNode_)
474-
dynamicInstanceTypeStack.push(InstanceTypeStackElement(typeFullName, mutable.HashSet.empty))
475474
typeRefIdStack.push(typeRefNode_)
475+
scope.pushNewTypeDeclScope(typeFullName)
476476
scope.pushNewMethodScope(typeFullName, typeName, typeDeclNode_, None)
477477

478478
val allClassMembers = declMembers(node, withConstructor = false).toList
@@ -491,9 +491,9 @@ trait AstForDeclSyntaxCreator(implicit withSchemaValidation: ValidationMode) {
491491
val staticMemberInits = allClassMembers.filter(isStaticMember)
492492

493493
methodAstParentStack.pop()
494-
dynamicInstanceTypeStack.pop()
495494
typeRefIdStack.pop()
496495
scope.popScope()
496+
scope.popScope()
497497

498498
if (staticMemberInits.nonEmpty) {
499499
val init = staticInitMethodAstAndBlock(
@@ -1016,7 +1016,7 @@ trait AstForDeclSyntaxCreator(implicit withSchemaValidation: ValidationMode) {
10161016
Ast(patternIdentifier)
10171017
} else {
10181018
// TODO: handle static members
1019-
val selfTpe = typeHintForSelfExpression().headOption.getOrElse(Defines.Any)
1019+
val selfTpe = typeForSelfExpression()
10201020
val baseNode = identifierNode(node, "self", "self", selfTpe)
10211021
scope.addVariableReference("self", baseNode, selfTpe, EvaluationStrategies.BY_REFERENCE)
10221022
fieldAccessAst(node, node, Ast(baseNode), s"self.$name", name, typeFullName)

‎joern-cli/frontends/swiftsrc2cpg/src/main/scala/io/joern/swiftsrc2cpg/astcreation/AstForExprSyntaxCreator.scala‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ trait AstForExprSyntaxCreator(implicit withSchemaValidation: ValidationMode) {
241241
callee match {
242242
case m: MemberAccessExprSyntax if m.base.isEmpty || code(m.base.get) == "self" =>
243243
// referencing implicit self
244-
val selfTpe = typeHintForSelfExpression().headOption.getOrElse(Defines.Any)
244+
val selfTpe = typeForSelfExpression()
245245
val selfNode = identifierNode(node, "self", "self", selfTpe)
246246
scope.addVariableReference("self", selfNode, selfTpe, EvaluationStrategies.BY_REFERENCE)
247247
handleCallNodeArgs(node, Ast(selfNode), code(m.declName.baseName))
@@ -256,7 +256,7 @@ trait AstForExprSyntaxCreator(implicit withSchemaValidation: ValidationMode) {
256256
case other if isRefToClosure(node, other) =>
257257
astForClosureCall(node)
258258
case declReferenceExprSyntax: DeclReferenceExprSyntax if code(declReferenceExprSyntax) != "self" =>
259-
val selfTpe = typeHintForSelfExpression().headOption.getOrElse(Defines.Any)
259+
val selfTpe = typeForSelfExpression()
260260
val selfNode = identifierNode(declReferenceExprSyntax, "self", "self", selfTpe)
261261
scope.addVariableReference(selfNode.name, selfNode, selfTpe, EvaluationStrategies.BY_REFERENCE)
262262
handleCallNodeArgs(node, Ast(selfNode), calleeCode)
@@ -389,7 +389,7 @@ trait AstForExprSyntaxCreator(implicit withSchemaValidation: ValidationMode) {
389389
val baseAst = base match {
390390
case None =>
391391
// referencing implicit self
392-
val selfTpe = typeHintForSelfExpression().headOption.getOrElse(Defines.Any)
392+
val selfTpe = typeForSelfExpression()
393393
val baseNode = identifierNode(node, "self", "self", selfTpe)
394394
scope.addVariableReference("self", baseNode, selfTpe, EvaluationStrategies.BY_REFERENCE)
395395
Ast(baseNode)

‎joern-cli/frontends/swiftsrc2cpg/src/main/scala/io/joern/swiftsrc2cpg/astcreation/AstNodeBuilder.scala‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,8 @@ trait AstNodeBuilder(implicit withSchemaValidation: ValidationMode) { this: AstC
138138
callAst(callNode_, arguments)
139139
}
140140

141-
protected def typeHintForSelfExpression(): Seq[String] = {
142-
dynamicInstanceTypeStack.headOption match {
143-
case Some(InstanceTypeStackElement(tpe, _)) => Seq(tpe)
144-
case None => methodAstParentStack.collectFirst { case t: NewTypeDecl => t.fullName }.toSeq
145-
}
141+
protected def typeForSelfExpression(): String = {
142+
scope.getEnclosingTypeDeclFullName.getOrElse(Defines.Any)
146143
}
147144

148145
protected def identifierNode(node: SwiftNode, name: String): NewIdentifier = {
@@ -153,7 +150,7 @@ trait AstNodeBuilder(implicit withSchemaValidation: ValidationMode) { this: AstC
153150
t
154151
}
155152
.getOrElse(name match {
156-
case "self" | "Self" => typeHintForSelfExpression().headOption.getOrElse(Defines.Any)
153+
case "self" | "Self" => typeForSelfExpression()
157154
case _ => Defines.Any
158155
})
159156
identifierNode(node, name, name, tpe)

‎joern-cli/frontends/swiftsrc2cpg/src/test/scala/io/joern/swiftsrc2cpg/passes/ast/SimpleAstCreationPassTest.scala‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,9 @@ class SimpleAstCreationPassTest extends AstSwiftSrc2CpgSuite {
258258

259259
"have correct structure for implicit self access with shadowing a variable from outer scope" in {
260260
val cpg = code("""
261-
|let f: String = "f"
261+
|let f: String = "a"
262262
|class Foo {
263-
| let f: String = "f"
263+
| let f: String = "b"
264264
| func bar() {
265265
| handleF(f)
266266
| }
@@ -276,6 +276,53 @@ class SimpleAstCreationPassTest extends AstSwiftSrc2CpgSuite {
276276
}
277277
}
278278

279+
"have correct structure for implicit self access from within nested functions" in {
280+
val cpg = code("""
281+
|let f: String = "a"
282+
|class Foo {
283+
| let f: String = "b"
284+
| func foo() -> String {
285+
| func closure() -> String {
286+
| return handleF(f)
287+
| }
288+
| return closure()
289+
| }
290+
|}
291+
|""".stripMargin)
292+
cpg.typeDecl.nameExact("Foo").member.name.l shouldBe List("f")
293+
val List(bAccess) = cpg.call.nameExact("handleF").argument.fieldAccess.l
294+
bAccess.code shouldBe "self.f"
295+
inside(bAccess.argument.l) { case List(id: Identifier, fieldName: FieldIdentifier) =>
296+
id.name shouldBe "self"
297+
id.typeFullName shouldBe "Test0.swift:<global>.Foo"
298+
fieldName.canonicalName shouldBe "f"
299+
}
300+
}
301+
302+
"have correct structure for implicit self access from within nested functions with shadowing" in {
303+
val cpg = code("""
304+
|let f: String = "a"
305+
|class Foo {
306+
| let f: String = "b"
307+
| func foo() -> String {
308+
| let f: String = "c"
309+
| func closure() -> String {
310+
| return handleF(f)
311+
| }
312+
| return closure()
313+
| }
314+
|}
315+
|""".stripMargin)
316+
cpg.typeDecl.nameExact("Foo").member.name.l shouldBe List("f")
317+
inside(cpg.call.nameExact("handleF").argument(1).l) { case List(id: Identifier) =>
318+
id.name shouldBe "f"
319+
id.typeFullName shouldBe Defines.String
320+
val fLocal = id._localViaRefOut.get
321+
fLocal.name shouldBe "f"
322+
fLocal.typeFullName shouldBe Defines.String
323+
}
324+
}
325+
279326
"have correct structure for implicit self access except for variable in closure" in {
280327
val cpg = code("""
281328
|class Foo {

0 commit comments

Comments
 (0)