Skip to content

Commit ac30297

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

File tree

4 files changed

+68
-31
lines changed

4 files changed

+68
-31
lines changed

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

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -164,28 +164,15 @@ trait AstCreatorHelper(implicit withSchemaValidation: ValidationMode) { this: As
164164

165165
protected def astForIdentifier(node: SwiftNode): Ast = {
166166
val identifierName = code(node)
167-
// scoping rules for self access:
168-
// 1) if identifier is in local method scope it shadows everything else:
169-
if (scope.variableIsInMethodScope(identifierName)) {
170-
val identNode = identifierNode(node, identifierName)
171-
val variableOption = scope.lookupVariable(identifierName)
172-
val tpe = variableOption match {
173-
case Some((_, variableTypeName)) if variableTypeName != Defines.Any => variableTypeName
174-
case None if identNode.typeFullName != Defines.Any => identNode.typeFullName
175-
case _ => Defines.Any
176-
}
177-
identNode.typeFullName = tpe
178-
scope.addVariableReference(identifierName, identNode, tpe, EvaluationStrategies.BY_REFERENCE)
179-
Ast(identNode)
180-
} else if (scope.variableIsInTypeDeclScope(identifierName)) {
181-
// 2) we found it as member of the surrounding type decl:
167+
if (scope.variableIsInTypeDeclScope(identifierName)) {
168+
// we found it as member of the surrounding type decl
182169
// (Swift does not allow to access any member / function of the outer class instance)
183170
val tpe = typeForSelfExpression()
184171
val selfNode = identifierNode(node, "self", "self", tpe)
185172
scope.addVariableReference("self", selfNode, selfNode.typeFullName, EvaluationStrategies.BY_REFERENCE)
186173
fieldAccessAst(node, node, Ast(selfNode), s"self.$identifierName", identifierName, tpe)
187174
} else {
188-
// 3) default handling, it must come from the outer scope somewhere
175+
// otherwise it must come from the outer scope somewhere
189176
val identNode = identifierNode(node, identifierName)
190177
val variableOption = scope.lookupVariable(identifierName)
191178
val tpe = variableOption match {

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

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import io.joern.x2cpg.frontendspecific.swiftsrc2cpg.Defines
88
import io.joern.x2cpg.{Ast, ValidationMode}
99
import io.shiftleft.codepropertygraph.generated.*
1010
import io.shiftleft.codepropertygraph.generated.nodes.*
11+
import io.shiftleft.semanticcpg.language.types.structure.NamespaceTraversal
1112

12-
import scala.collection.mutable
1313
import scala.annotation.unused
1414

1515
trait AstForDeclSyntaxCreator(implicit withSchemaValidation: ValidationMode) {
@@ -612,8 +612,10 @@ trait AstForDeclSyntaxCreator(implicit withSchemaValidation: ValidationMode) {
612612
val MethodInfo(methodName, methodFullName, signature, returnType) = methodInfo
613613
val methodFullNameAndSignature = methodInfo.fullNameAndSignature
614614

615-
val shouldCreateFunctionReference =
616-
typeRefIdStack.isEmpty || node.isInstanceOf[ClosureExprSyntax] || node.isInstanceOf[AccessorDeclSyntax]
615+
val shouldCreateFunctionReference = typeRefIdStack.isEmpty ||
616+
methodAstParentStack.headOption.exists(_.isInstanceOf[NewMethod]) ||
617+
node.isInstanceOf[ClosureExprSyntax] ||
618+
node.isInstanceOf[AccessorDeclSyntax]
617619
val methodRefNode_ = if (!shouldCreateFunctionReference) { None }
618620
else { Option(methodRefNode(node, methodName, methodFullNameAndSignature, methodFullNameAndSignature)) }
619621
val capturingRefNode = methodRefNode_.orElse(typeRefIdStack.headOption)
@@ -629,12 +631,13 @@ trait AstForDeclSyntaxCreator(implicit withSchemaValidation: ValidationMode) {
629631

630632
val parameterAsts = node match {
631633
case f: FunctionDeclSyntax =>
632-
val selfAst = if (isStaticMember(f)) {
633-
val parameterNode =
634-
parameterInNode(node, "self", "self", 0, false, EvaluationStrategies.BY_SHARING, parentFullName)
635-
scope.addVariable("self", parameterNode, parentFullName, VariableScopeManager.ScopeType.MethodScope)
636-
Seq(Ast(parameterNode))
637-
} else Seq.empty
634+
val selfAst =
635+
if (!isStaticMember(f) && !typeForSelfExpression().endsWith(NamespaceTraversal.globalNamespaceName)) {
636+
val parameterNode =
637+
parameterInNode(node, "self", "self", 0, false, EvaluationStrategies.BY_SHARING, parentFullName)
638+
scope.addVariable("self", parameterNode, parentFullName, VariableScopeManager.ScopeType.MethodScope)
639+
Seq(Ast(parameterNode))
640+
} else Seq.empty
638641
selfAst ++ f.signature.parameterClause.parameters.children.map(astForNode)
639642
case a: AccessorDeclSyntax =>
640643
val parameterNode =
@@ -657,10 +660,17 @@ trait AstForDeclSyntaxCreator(implicit withSchemaValidation: ValidationMode) {
657660
scope.addVariable("self", parameterNode, parentFullName, VariableScopeManager.ScopeType.MethodScope)
658661
Ast(parameterNode) +: s.parameterClause.parameters.children.map(astForNode)
659662
case c: ClosureExprSyntax =>
660-
c.signature.flatMap(_.parameterClause) match
663+
val selfAst = if (!typeForSelfExpression().endsWith(NamespaceTraversal.globalNamespaceName)) {
664+
val parameterNode =
665+
parameterInNode(node, "self", "self", 0, false, EvaluationStrategies.BY_SHARING, parentFullName)
666+
scope.addVariable("self", parameterNode, parentFullName, VariableScopeManager.ScopeType.MethodScope)
667+
Seq(Ast(parameterNode))
668+
} else Seq.empty
669+
selfAst ++ (c.signature.flatMap(_.parameterClause) match {
661670
case Some(p: ClosureShorthandParameterListSyntax) => p.children.map(astForNode)
662671
case Some(p: ClosureParameterClauseSyntax) => p.parameters.children.map(astForNode)
663672
case None => Seq.empty
673+
})
664674
}
665675

666676
val body: Option[CodeBlockSyntax | AccessorDeclListSyntax | CodeBlockItemListSyntax] = node match {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import io.joern.x2cpg.frontendspecific.swiftsrc2cpg.Defines
88
import io.joern.x2cpg.{Ast, ValidationMode}
99
import io.shiftleft.codepropertygraph.generated.*
1010
import io.shiftleft.codepropertygraph.generated.nodes.NewCall
11+
import io.shiftleft.semanticcpg.language.types.structure.NamespaceTraversal
1112

1213
import scala.annotation.unused
1314

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

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -282,10 +282,10 @@ class SimpleAstCreationPassTest extends AstSwiftSrc2CpgSuite {
282282
|class Foo {
283283
| let f: String = "b"
284284
| func foo() -> String {
285-
| func closure() -> String {
285+
| func bar() -> String {
286286
| return handleF(f)
287287
| }
288-
| return closure()
288+
| return bar()
289289
| }
290290
|}
291291
|""".stripMargin)
@@ -297,6 +297,15 @@ class SimpleAstCreationPassTest extends AstSwiftSrc2CpgSuite {
297297
id.typeFullName shouldBe "Test0.swift:<global>.Foo"
298298
fieldName.canonicalName shouldBe "f"
299299
}
300+
301+
val List(fooMethod) = cpg.method.nameExact("foo").l
302+
val List(fooBlock) = fooMethod.astChildren.isBlock.l
303+
val List(barRef) = fooBlock.astChildren.isMethodRef.l
304+
barRef.captureOut shouldBe empty
305+
306+
val List(barMethod) = cpg.method.nameExact("bar").l
307+
val List(barMethodBlock) = barMethod.astChildren.isBlock.l
308+
barMethodBlock.astChildren.isLocal.name("f") shouldBe empty
300309
}
301310

302311
"have correct structure for implicit self access from within nested functions with shadowing" in {
@@ -306,10 +315,10 @@ class SimpleAstCreationPassTest extends AstSwiftSrc2CpgSuite {
306315
| let f: String = "b"
307316
| func foo() -> String {
308317
| let f: String = "c"
309-
| func closure() -> String {
318+
| func bar() -> String {
310319
| return handleF(f)
311320
| }
312-
| return closure()
321+
| return bar()
313322
| }
314323
|}
315324
|""".stripMargin)
@@ -321,6 +330,22 @@ class SimpleAstCreationPassTest extends AstSwiftSrc2CpgSuite {
321330
fLocal.name shouldBe "f"
322331
fLocal.typeFullName shouldBe Defines.String
323332
}
333+
val List(fooMethod) = cpg.method.nameExact("foo").l
334+
val List(fooBlock) = fooMethod.astChildren.isBlock.l
335+
val List(fooLocalF) = fooBlock.astChildren.isLocal.nameExact("f").l
336+
val List(barRef) = fooBlock.astChildren.isMethodRef.l
337+
val List(closureBindingF) = barRef.captureOut.l
338+
closureBindingF.closureBindingId shouldBe Option("Test0.swift:<global>.Foo.foo.bar:f")
339+
closureBindingF.refOut.head shouldBe fooLocalF
340+
closureBindingF.evaluationStrategy shouldBe EvaluationStrategies.BY_REFERENCE
341+
342+
val List(barMethod) = cpg.method.nameExact("bar").l
343+
val List(barMethodBlock) = barMethod.astChildren.isBlock.l
344+
val List(barLocal) = barMethodBlock.astChildren.isLocal.name("f").l
345+
barLocal.closureBindingId shouldBe Option("Test0.swift:<global>.Foo.foo.bar:f")
346+
347+
val List(identifierF) = barMethodBlock.ast.isIdentifier.nameExact("f").l
348+
identifierF.refOut.head shouldBe barLocal
324349
}
325350

326351
"have correct structure for implicit self access except for variable in closure" in {
@@ -329,7 +354,7 @@ class SimpleAstCreationPassTest extends AstSwiftSrc2CpgSuite {
329354
| let f: String = "f"
330355
| class Bar {
331356
| let b = "b"
332-
| func bar() {
357+
| func foo() {
333358
| let compare = { (s1: String, s2: String) -> Bool in
334359
| let f: Int = 1
335360
| handleB(b)
@@ -356,6 +381,20 @@ class SimpleAstCreationPassTest extends AstSwiftSrc2CpgSuite {
356381
fLocal.name shouldBe "f"
357382
fLocal.typeFullName shouldBe Defines.Int
358383
}
384+
385+
val List(fooMethod) = cpg.method.nameExact("foo").l
386+
val List(fooBlock) = fooMethod.astChildren.isBlock.l
387+
val List(compareRef) = fooBlock.ast.isMethodRef.l
388+
compareRef.captureOut shouldBe empty
389+
390+
val List(compareClosure) = cpg.method.nameExact("<lambda>0").l
391+
val List(compareClosureBlock) = compareClosure.astChildren.isBlock.l
392+
val List(compareClosureLocal) = compareClosureBlock.astChildren.isLocal.name("f").l
393+
val List(identifierFFromLocalDecl, identifierFInCall) = compareClosureBlock.ast.isIdentifier.nameExact("f").l
394+
identifierFFromLocalDecl.refOut.head shouldBe compareClosureLocal
395+
identifierFFromLocalDecl.lineNumber shouldBe Some(8)
396+
identifierFInCall.refOut.head shouldBe compareClosureLocal
397+
identifierFInCall.lineNumber shouldBe Some(10)
359398
}
360399

361400
"have correct structure for implicit self access except for parameters" in {

0 commit comments

Comments
 (0)